Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Encode path segments in urls to prevent illegal uris #124

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jesseeichar
Copy link

I have a geoserver instance where users can publish layers to the geoserver. Because there are so many users that can publish to the geoserver we have been unable to prevent them from publishing layers with spaces in the names. When there are spaces (or other certain characters), the rest api
is broken because the urls have spaces in them.

Obviously the best solution is to fix Geoserver so that a layer with a space in the name (or in the workspace name) does not break the REST API. However that is a substantial amount of work and this change makes this library more robust in the face or inconsiderate geoserver
administrators so I decided to make the change here. When I have time I would like to patch Geoserver as well but that is another issue.

The fix I have made is quite simple. In HttpUtils I separate each url into its path segments and encode each segment then put the url back together again. Because of this all urls will work regardless of the workspace or layer name.

@ccancellieri
Copy link
Contributor

Jesse,
thanks for the patch.
It seems quite simple and we could merge it soon but I'm not sure about the:
jesseeichar@3a73377#diff-ea2d6faa1ff5d0d9825e13de46e99d59R122

I think we may try to solve the problem definitively using the URI (
http://docs.oracle.com/javase/7/docs/api/java/net/URI.html) class.
which is able to correctly encode the URI.

What do you think?

Cheers,
Carlo

2014-04-24 8:45 GMT+02:00 Jesse Eichar [email protected]:

I have a geoserver instance where users can publish layers to the
geoserver. Because there are so many users that can publish to the
geoserver we have been unable to prevent them from publishing layers with
spaces in the names. When there are spaces (or other certain characters),
the rest api
is broken because the urls have spaces in them.

Obviously the best solution is to fix Geoserver so that a layer with a
space in the name (or in the workspace name) does not break the REST API.
However that is a substantial amount of work and this change makes this
library more robust in the face or inconsiderate geoserver
administrators so I decided to make the change here. When I have time I
would like to patch Geoserver as well but that is another issue.

The fix I have made is quite simple. In HttpUtils I separate each url into
its path segments and encode each segment then put the url back together
again. Because of this all urls will work regardless of the workspace or

layer name.

You can merge this Pull Request by running

git pull https://github.com/jesseeichar/geoserver-manager master

Or view, comment on, or merge it at:

#124
Commit Summary

  • Encode path segments in urls to prevent illegal uris thanks to layer
    names with spaces.

File Changes

Patch Links:


Reply to this email directly or view it on GitHubhttps://github.com//pull/124
.

Dott. carlo cancellieri
skype: ccancellieri
Twitter: @cancellieric
LinkedIn: http://it.linkedin.com/in/ccancellieri/

@jesseeichar
Copy link
Author

It was I admit a fairly quick hack I put together to solve my probably. I
think you are correct that it should be changed to use the URI class. I
will make that change as soon as I have time.

Jesse

On Mon, Apr 28, 2014 at 11:57 AM, Carlo Cancellieri <
[email protected]> wrote:

Jesse,
thanks for the patch.
It seems quite simple and we could merge it soon but I'm not sure about
the:

jesseeichar@3a73377#diff-ea2d6faa1ff5d0d9825e13de46e99d59R122

I think we may try to solve the problem definitively using the URI (
http://docs.oracle.com/javase/7/docs/api/java/net/URI.html) class.
which is able to correctly encode the URI.

What do you think?

Cheers,
Carlo

2014-04-24 8:45 GMT+02:00 Jesse Eichar [email protected]:

I have a geoserver instance where users can publish layers to the
geoserver. Because there are so many users that can publish to the
geoserver we have been unable to prevent them from publishing layers
with
spaces in the names. When there are spaces (or other certain
characters),
the rest api
is broken because the urls have spaces in them.

Obviously the best solution is to fix Geoserver so that a layer with a
space in the name (or in the workspace name) does not break the REST
API.
However that is a substantial amount of work and this change makes this
library more robust in the face or inconsiderate geoserver
administrators so I decided to make the change here. When I have time I
would like to patch Geoserver as well but that is another issue.

The fix I have made is quite simple. In HttpUtils I separate each url
into
its path segments and encode each segment then put the url back together
again. Because of this all urls will work regardless of the workspace or

layer name.

You can merge this Pull Request by running

git pull https://github.com/jesseeichar/geoserver-manager master

Or view, comment on, or merge it at:

#124
Commit Summary

  • Encode path segments in urls to prevent illegal uris thanks to layer
    names with spaces.

File Changes

Patch Links:


Reply to this email directly or view it on GitHub<
https://github.com/geosolutions-it/geoserver-manager/pull/124>
.

Dott. carlo cancellieri
skype: ccancellieri
Twitter: @cancellieric
LinkedIn: http://it.linkedin.com/in/ccancellieri/


Reply to this email directly or view it on GitHubhttps://github.com//pull/124#issuecomment-41541138
.

@ccancellieri
Copy link
Contributor

great, thanks.
Carlo

…names with spaces.

I have a geoserver instance where users can publish layers to the geoserver.  Because there are so many users that can publish to the geoserver we have been unable to prevent them from publishing layers with spaces in the names.  When there are spaces (or other certain characters), the rest api
is broken because the urls have spaces in them.

Obviously the best solution is to fix Geoserver so that a layer with a space in the name (or in the workspace name) does not break the REST API.  However that is a substantial amount of work and this change makes this library more robust in the face or inconsiderate geoserver
administrators so I decided to make the change here.  When I have time I would like to patch Geoserver as well but that is another issue.
@jesseeichar
Copy link
Author

I have made the suggested change. Now new URI(...) is used to construct a correctly escaped URI. Kind of silly I didn't think of that the first time because I had done just that perhaps a day before writing the first draft.

@ccancellieri
Copy link
Contributor

Jesse,

I have made the suggested change. Now new URI(...) is used to construct a
correctly escaped URI. Kind of silly I

cool!

didn't think of that the first time because I had done just that perhaps a
day before writing the first draft.

No, problem, this is why we usually discuss of patches on the ML before
start coding ;)

Well, I'm waiting for an updated pull req to merge.
Cheers,
Carlo


Reply to this email directly or view it on GitHubhttps://github.com//pull/124#issuecomment-42274402
.

Dott. carlo cancellieri
skype: ccancellieri
Twitter: @cancellieric
LinkedIn: http://it.linkedin.com/in/ccancellieri/

@jesseeichar
Copy link
Author

When I did a git amend and forced pushed the change so when I looked at the
pull request it looked like the pull request was already updated. The only
change was the the encodeUrl method.

Because I did an amend you can't really see the changes I made just the new
version. I thought doing it this way would make the history of
geoserver-manager look cleaner.

Jesse

On Tue, May 6, 2014 at 10:16 AM, Carlo Cancellieri <[email protected]

wrote:

Jesse,

I have made the suggested change. Now new URI(...) is used to construct
a
correctly escaped URI. Kind of silly I

cool!

didn't think of that the first time because I had done just that perhaps
a
day before writing the first draft.

No, problem, this is why we usually discuss of patches on the ML before
start coding ;)

Well, I'm waiting for an updated pull req to merge.
Cheers,
Carlo


Reply to this email directly or view it on GitHub<
https://github.com/geosolutions-it/geoserver-manager/pull/124#issuecomment-42274402>

.

Dott. carlo cancellieri
skype: ccancellieri
Twitter: @cancellieric
LinkedIn: http://it.linkedin.com/in/ccancellieri/

Reply to this email directly or view it on GitHubhttps://github.com//pull/124#issuecomment-42277093
.

@ccancellieri
Copy link
Contributor

Hi,

When I did a git amend and forced pushed the change so when I looked at the
pull request it looked like the pull request was already updated. The only
change was the the encodeUrl method.

Oh, yes, sorry I didn't catch the changes.

I'm not sure why you are manually split the url, I think we can just replace the call to the encodeUrl method with the "new URI(String )" constructor.
Take a look at the URI code:
http://grepcode.com/file/repository.grepcode.com/java/root/jdk/openjdk/6-b14/java/net/URI.java#URI.%3Cinit%3E%28java.lang.String%29

don't you think so?

@jesseeichar
Copy link
Author

I can't just use new URI because it will give uri syntax exceptions. The
following lines both result in URI exceptions:

new URI("http://path with spaces")
new URI("http://path/with spaces")

The encoding only correctly occurs when you break the URL apart and use one
of the constructors:

new URI(protocol, authority, path, query, fragment)

or

new URI(protocol, host, port, path, query, fragment)

I wish I could use new URI(String) that would have made my life much
easier :(.

Jesse

On Tue, May 6, 2014 at 11:06 AM, Carlo Cancellieri <[email protected]

wrote:

Hi,

When I did a git amend and forced pushed the change so when I looked at the
pull request it looked like the pull request was already updated. The only
change was the the encodeUrl method.

Oh, yes, sorry I didn't catch the changes.

I'm not sure why you are manually split the url, I think we can just
replace the call to the encodeUrl method with the "new URI(String )"
constructor.
Take a look at the URI code:

http://grepcode.com/file/repository.grepcode.com/java/root/jdk/openjdk/6-b14/java/net/URI.java#URI.%3Cinit%3E%28java.lang.String%29

don't you think so?


Reply to this email directly or view it on GitHubhttps://github.com//pull/124#issuecomment-42280725
.

@ccancellieri
Copy link
Contributor

ok, thanks for clarifying it.
Waiting for feedback by Emanuele.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants