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

Implicitly Decodes Encoded URIs #7

Closed
dvisco opened this Issue Nov 15, 2016 · 3 comments

Comments

Projects
None yet
2 participants
@dvisco
Copy link

dvisco commented Nov 15, 2016

Not sure if this is to be expected but the HttpRequest.parseUri method is using the getPath() method of java.net.URI instead of getRawPath(). This presents a problem in two places:

  1. In AbstractHttpClient.run the request is cloned which triggers another call to HttpRequest.parseUri from HttpRequest.setUri on the new Cloned Object but this will receive the decoded path. This will throw an exception if the original uri required encoding to prevent a URISyntaxException from occuring
  2. Also within JerseyHttpClient.doExecute the client.resource call is passed the decoded URI which will throw an exception as well if the decoded URI required encoding.

Maybe I am missing something but I can't see how else to get around the issue given the API provided. I ended up having to extend the HttpRequest class and override the calls to clone and parseUri while using reflection to access the uri field.

@budjb budjb added this to the 2.0 milestone Jul 23, 2018

@budjb budjb self-assigned this Jul 23, 2018

@budjb

This comment has been minimized.

Copy link
Owner

budjb commented Jul 29, 2018

I know it's been a while for this issue, but do you have an example to reproduce? I'm looking at this for 2.0, and I cannot reproduce the issue (at least the one specific to cloning).

I've tried the following, still using .getPath():

def 'this should fail, but it does not'() {
    setup:
    HttpRequest request = new HttpRequest('http://localhost/the+bads')

    when:
    request = (HttpRequest) request.clone()

    then:
    request.getUri() == 'http://localhost/the+bads'
}

Maybe some of the restructuring has fixes this, but the request object still clones itself the same way, so I'd think it would behave the same. Am I missing something?

budjb added a commit that referenced this issue Jul 29, 2018

@budjb

This comment has been minimized.

Copy link
Owner

budjb commented Jul 29, 2018

Digging through the code, I found the issue is caused when paths have character escaping (encoding starts with '%'). I reproduced the issue and submitted a fix that will be present in 2.0. Thanks for the report!

@budjb budjb closed this Jul 29, 2018

@dvisco

This comment has been minimized.

Copy link

dvisco commented Jul 30, 2018

No problem thanks for the lib!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment