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

Support absolute URLs #219

Closed
jorgemarey opened this issue Jan 20, 2017 · 8 comments
Closed

Support absolute URLs #219

jorgemarey opened this issue Jan 20, 2017 · 8 comments
Milestone

Comments

@jorgemarey
Copy link

Hi!

I was using fabio as a proxy to some of my apps, everything is working great, but I notice something weird.

Fabio wasn't able to proxy some requests. I went to the logs and saw the following:

[WARN] No route for  myservice.myproxy.mydomainhttps://myservice.myproxy.mydomain/somepath

I did some research and found that the service asking is setting the rquest.URL.Opaque field (it's a golang service). This makes the RequestURI on the server side include coplete URL.

Found this on the golang net/http package:

// RequestURI is the unmodified Request-URI of the
// Request-Line (RFC 2616, Section 5.1) as sent by the client
// to a server. Usually the URL field should be used instead.
// It is an error to set this field in an HTTP client request.
RequestURI string

I found that changing the references to req.RequestURI to req.URL.RequestURI on https://github.com/eBay/fabio/blob/v1.3.7/route/table.go#L284 makes the service request match correctly and didn't break anything else (or at list I think so).

I ran the test but some of them failed (TestTableLookup and TestIssue57. Both with panic: runtime error: invalid memory address or nil pointer dereference). For what I could see, in those test the RequestURI field of the Request struct was filled directly, so that failure it's normal.

I don't know if this too core to be changed, or if I explained myself clearly. If you think this could be changed I could try and make a PR.

Thanks!

@magiconair
Copy link
Contributor

Thanks for reporting this. I will have a look at this. Can you provide an example of a raw HTTP request that triggers that behavior?

@jorgemarey
Copy link
Author

jorgemarey commented Jan 20, 2017

Yes.

I run tcpdump on the machine where fabio is running (chaning the protocol to http) and saw the next:
In my case I have a s3 compatible service behind fabio.
Using the aws-sdk-go

This happens when I use the aws-sdk-go

GET http://s3.mydomain.com/mybucket?delimiter=%2F&max-keys=1000&prefix= HTTP/1.1
Host: s3.mydomain.com
....other headers....

This when I use curl

GET /mybucket?delimiter=%2F&max-keys=1000&prefix= HTTP/1.1
Host: s3.mydomain.com
....other headers....

@magiconair
Copy link
Contributor

OK, so the RequestURI contains an absolute URL. If I read https://www.w3.org/Protocols/rfc2616/rfc2616-sec5.html correctly, then clients should send this only to proxies but servers should always handle this.

magiconair added a commit that referenced this issue Jan 20, 2017
This patch adds support for requests with absolute URLs.
According to https://www.w3.org/Protocols/rfc2616/rfc2616-sec5.html
servers must accept requests with absolute URLs.
@magiconair magiconair changed the title RequestURI match problem Support absolute URLs Jan 20, 2017
@magiconair
Copy link
Contributor

@jorgemarey This patch should fix that. Could you test that, please? It just occurred to me that I also need to extend the test cases to cover that case. I'll do that later today before I merge the fix.

@jorgemarey
Copy link
Author

That was quick! I tried it. It works perfectly fine!

@magiconair
Copy link
Contributor

Well, you've done the heavy lifting. Thanks a lot for that. Can you confirm that it works for both types of requests: the ones with and without an absolute URL?

@jorgemarey
Copy link
Author

Glad to help! Yes, I tried with both types of requests and It worked for both of them.

@magiconair
Copy link
Contributor

Merged to master.

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

No branches or pull requests

2 participants