proxy: handle encoded path in URL #1378

Merged
merged 1 commit into from Feb 17, 2017

Projects

None yet

2 participants

@tw4452852
Collaborator

fix issue #1362

Signed-off-by: Tw tw19881113@gmail.com

+ return a + "/" + b
+ }
+ return a + b
+}
@mholt
mholt Feb 8, 2017 edited Owner

Is this different from path.Join? I know it will strip off a trailing / but maybe there is a way to simplify this change?

@tw4452852
tw4452852 Feb 9, 2017 Collaborator

path.Join will also strip the middle slash: /abc/ + /de = abc/de instead of abc//de. This case will often occur in the encoded URL and it is the way that standard lib uses.

@mholt
Owner
mholt commented Feb 8, 2017

Thanks for the PR, Tw! This seems like a complicated change for what looks like a simple problem. What about the encoded comma is causing the comma to drop path components?

@tw4452852
Collaborator
tw4452852 commented Feb 9, 2017 edited

I think doc will explain the everything.

@mholt

Thanks as usual @tw4452852! I think this is pretty close, I mostly want some comments/documentation. Also, do you think this issue or PR is related to #1424 in any way?

caddyhttp/proxy/reverseproxy.go
+ b = req.URL.Path
+ }
+
+ req.URL.RawPath = singleJoiningSlash(a, b)
@mholt
mholt Feb 16, 2017 Owner

I think I'm OK with this logic, but could you comment the relevant parts, like why the if statements are the way they are, or what the point of these if statements is? Maybe this chunk belongs in a separate function.

@tw4452852 tw4452852 proxy: handle encoded path in URL
fix issue #1362

Signed-off-by: Tw <tw19881113@gmail.com>
c37481c
@tw4452852
Collaborator

@mholt Got it. I have revised it a bit. Wish it looks promising now.

@mholt
mholt approved these changes Feb 17, 2017 View changes
@mholt
Owner
mholt commented Feb 17, 2017

Thanks Tw! Looks great. 👍

@mholt mholt merged commit e50de80 into mholt:master Feb 17, 2017

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
licence/cla Contributor License Agreement is signed.
Details
@tw4452852 tw4452852 deleted the tw4452852:1362 branch Feb 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment