search

boblemaire / asyncHTTPrequest

asynchronous HTTP for ESP using ESPasyncTCP. Works like XMLHTTPrequest in JS.
GNU General Public License v3.0
65 stars 31 forks source link

Not using absolute URLs for HTTP requests #14

Closed danielbuechele closed 4 years ago

danielbuechele commented 4 years ago

The request was sent like this:

GET http://myserver.tld/path?query HTTP/1.1

However, this is very uncommon and not recommended. In fact I have seen some problems with some HTTP servers and especially PHP handling requests like this. Looking at the HTTP protocol spec I found this:

The absoluteURI form is only allowed when the request is being made to a proxy. The most common form of Request-URI is that used to identify a resource on an origin server or gateway. In this case, only the absolute path of the URI is transmitted.

So, in this PR, I am changing the implementation, so the request from above looks like this: 

GET /path?query HTTP/1.1
Host: http://myserver.tld:80
boblemaire commented 4 years ago

Without getting into the merits of this change, it looks as if you are building a host: request header directly in the request xbuf, but not too far below that, all of the request headers are copied into the request. On line 84 in the open() function a host: request header has already been created. So with your change, you should be seeing two host: headers in your request stream.

The host: header created in open() does not have the port appended, but if it's needed, that would probably be the place to add it. It's not as simple there, but that's how request headers are handled.

That said, since the host header is in every transaction, I'd look the other way creating it there as long as the redundant header isn't created.

danielbuechele commented 4 years ago

You are right, thanks for pointing this out! I removed the duplicate Host: header. Still I think regarding to the HTTP spec, the first line or the request should not be an absolute URL.

I updated the PR accordingly.

joekane101 commented 4 years ago

Thank you Daniel, I was pulling my hair out trying to make the library work.

The simple http server on a network appliance I am trying to connect to kept sending 404 errors for my requests.
'This page does not exist".. So the server was definitely not liking the absolute path, and probably was trying to map to some relative one, which then didn't exist.

Once I incorporated this PR, the connections started working.

boblemaire commented 4 years ago

I've committed this PR, and will add the port to the host header as in the original PR before the redundant host header was removed.