-
Notifications
You must be signed in to change notification settings - Fork 512
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
User agent not set correctly when specifying a base url #290
Comments
you're right, and it also seems like "accept" header is missing too, looks like no headers are set on request, simple test reproducing this: Without baseurl there are "Accept" and "User-Agent" headers. |
Aha! This is because user_agent is set on web_page object and when we pass "base_url" we never load url into webpage but we bypass webpage completely |
hey @kmike @Youwotma seems like we dont set UA at all for all "manual" (not loaded via qwebpage) requests, e.g. if you do splash:http_get() or splash:http_post() request from Splash will not have UA. If you do go() request will have default Splash UA. Should we consider this a bug? By default if you use HTTP API or Lua splash:go() without base url you get UA: "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/538.1 (KHTML, like Gecko) splash Safari/538.1" so maybe we could set this as default UA for all requests. |
I think it makes sense to use default headers in http_get and http_post. |
and same for go with base url right? so we should have default UA for all those requests. I'm not sure how websites would handle reqeusts without any UA, perhaps no big deal but could be trouble in some cases? |
yeah, right |
and did you mean UA or all headers? Because QWebPage also adds "Accept: text/html" header, which is also missing |
I think Re other header: I think it is fine to use headers set by splash:set_custom_headers in http_get/http_post, but there should be a way to override it just for a single request. Currently we only adding or replacing headers using |
I see in docs that there is request:set_header(name, value) so that would probably be the way to do it? |
would be nice to have some settings where users could set default headers, but I see that #279 is stalled now, perhaps worth prioritizing it sounds like a really useful PR and lots of work already done |
Test script:
Uses
Mozilla/5.0
instead ofFooBar/1.0
.Specifying the user agent as a header works.
The text was updated successfully, but these errors were encountered: