After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 758922 - rtspconnection should optionally make HTTP requests with abs_path instead of absoluteURI
rtspconnection should optionally make HTTP requests with abs_path instead of ...
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
1.6.1
Other All
: Normal normal
: 1.7.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-12-01 19:54 UTC by Evan Callaway
Modified: 2015-12-14 17:22 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch to use relative URIs for HTTP tunning (5.19 KB, patch)
2015-12-04 18:19 UTC, Evan Callaway
none Details | Review
http tunneling relative uris (6.46 KB, patch)
2015-12-14 16:27 UTC, Evan Callaway
committed Details | Review

Description Evan Callaway 2015-12-01 19:54:17 UTC
Consider the following pipeline for streaming RTSP over HTTP:

gst-launch-1.0 rtspsrc location="rtsph://user:pass@192.168.105.64:80/rtsp_tunnel" short-header=true ! decodebin ! fakesink

Referencing http://www.w3.org/Protocols/HTTP/1.0/spec.html#Request-URI, with the current rtspconnection code we always specify the absoluteURI in our requests.  For example, a representative request would look like this:

POST http://192.168.105.64:80/rtsp_tunnel HTTP/1.0

This seems to work well for many servers, but there are a few very strict servers that require HTTP requests to go to abs_path:

POST /rtsp_tunnel HTTP/1.0

Obviously, using absoluteURI is required if you are using a proxy so that the request can be correctly routed.  I propose that we add a "use-abs-path" flag to rtspsrc which in turn would instruct rtspconnection to use abs_path instead of absoluteURI for HTTP requests for compatibility for these strict servers.

Please let me know your thoughts on this.  I'm happy to provide a patch if this seems like an agreeable solution.

This issue would also be resolved by using HTTP/1.1 with the Host header as it would be correctly parsed.
Comment 1 Sebastian Dröge (slomo) 2015-12-02 07:16:53 UTC
I think implementing HTTP/1.1 support would be preferred here, and if using that fails a fallback to HTTP/1.0 should be done that does what the current code does.


Additionally it might make sense to only do an absolute request if really needed, that is when the non-path part of the URI would differ.


And additionally there is also the x-server-ip-address header, which we should probably handle here.
Comment 2 Evan Callaway 2015-12-02 19:14:38 UTC
I agree, going forward using HTTP/1.1 would be the better solution.  Naively, HTTP/1.1 was supposed to be backwards compatible with 1.0, though I would bet that in practice there will be servers that strictly require 1.0.

It's my understanding that you're correct-- we should favor the abs_path except when required in HTTP/1.0.  With 1.1 requests we should be able to always use abs_path combined with the Host header as it's possible to reconstruct the full path.  I will have to review the spec to be certain.
Comment 3 Sebastian Dröge (slomo) 2015-12-03 08:50:32 UTC
(In reply to Evan Callaway from comment #2)
> I agree, going forward using HTTP/1.1 would be the better solution. 
> Naively, HTTP/1.1 was supposed to be backwards compatible with 1.0, though I
> would bet that in practice there will be servers that strictly require 1.0.

It's only backwards compatible if you don't use any new HTTP/1.1 features... like the Host header :)

> It's my understanding that you're correct-- we should favor the abs_path
> except when required in HTTP/1.0.  With 1.1 requests we should be able to
> always use abs_path combined with the Host header as it's possible to
> reconstruct the full path.  I will have to review the spec to be certain.

Thanks, can you add a comment with the results of your research and ideally also provide a patch that implements this? Thanks!
Comment 4 Evan Callaway 2015-12-03 16:45:13 UTC
Okay, I believe I have a good understanding of it now.  In 5.1.2 the HTTP/1.1 spec it is fairly clear:

- The absoluteURI form is REQUIRED when the request is being made to a proxy
- HTTP/1.1 clients will only generate [absoluteURIs] in requests to proxies

So even though it would be non-ambiguous to use the shorter relative URI with a Host header, we are still supposed to use absolute URIs when talking to proxies, presumably for backwards compatibility with HTTP/1.0.

I will take a look at building out these changes.
Comment 5 Evan Callaway 2015-12-03 21:33:09 UTC
More results!  Reading the HTTP/1.0 spec, I've found that absoluteURI request form is only allowed when going to a proxy.  This fix is rather small and  doesn't require the complexity of implementing HTTP/1.1 with a fall-back to HTTP/1.0, and neatly resolves my issue.  Always nice to see it work out where there's a simpler solution to the problem.

I have to do more testing, but once things look solid I will provide a patch to make our HTTP/1.0 requests match spec.
Comment 6 Sebastian Dröge (slomo) 2015-12-04 06:56:15 UTC
Makes sense, thanks for looking into this!
Comment 7 Evan Callaway 2015-12-04 18:19:25 UTC
Created attachment 316783 [details] [review]
Patch to use relative URIs for HTTP tunning

Here's a patch that resolves this issue for me.

Note that this patch assumes that the HTTP tunneling authentication patch (https://bugzilla.gnome.org/show_bug.cgi?id=749596) has already been applied as they're both touching the same part of the code and I needed it for testing with some of my cameras.
Comment 8 Sebastian Dröge (slomo) 2015-12-09 09:50:15 UTC
Review of attachment 316783 [details] [review]:

Looks good to me.

Should we additionally add the Host header just in case? Make it use HTTP/1.1?
Because now we send less information in the request for non-tunnelling, which might break on some servers that would've previously worked.
Comment 9 Evan Callaway 2015-12-09 14:30:06 UTC
Sounds reasonable to add the Host header.  It's not officially documented, but many HTTP/1.0 servers make use of the Host header when it's available, and it shouldn't cause any trouble to add it as it is ignored by servers that don't support it.
Comment 10 Sebastian Dröge (slomo) 2015-12-11 08:18:14 UTC
Ok, are you going to update the patch accordingly?
Comment 11 Evan Callaway 2015-12-11 15:22:00 UTC
Yes, I will resubmit the patch with the change once I've had a chance to run it through its paces.
Comment 12 Sebastian Dröge (slomo) 2015-12-14 09:49:41 UTC
Great, thanks :)
Comment 13 Evan Callaway 2015-12-14 16:27:13 UTC
Created attachment 317382 [details] [review]
http tunneling relative uris

This patch adds the Host header to the HTTP requests that are used to setup tunneling, but it's still claiming HTTP/1.0 in the message header.  This is generally fine as the Host header is sort of an unofficial HTTP/1.0 header anywway.

If there is any danger of rtsp servers strictly parsing and for "HTTP/1.0" in the header and rejecting otherwise, we should leave it as is.  Otherwise, I think we can update the message header to claim "HTTP/1.1" as that's really what we're doing.  Let me know what you think and I will update this patch as necessary.
Comment 14 Sebastian Dröge (slomo) 2015-12-14 17:20:20 UTC
Review of attachment 317382 [details] [review]:

I think this makes sense as is. We don't *require* any HTTP 1.1 features, we only add additional hints via an extra header here. So I think it's fine to request HTTP 1.0 and just put the new header in. Unknown headers must be ignored by clients anyway.

Only if we actually required HTTP 1.1 features it would AFAIU be required that we ask for HTTP 1.1.
Comment 15 Sebastian Dröge (slomo) 2015-12-14 17:22:23 UTC
commit 5ac65d9e3a6fa43b884553d643dd1042f0ca79b2
Author: Evan Callaway <evan.callaway@ipconfigure.com>
Date:   Mon Dec 14 10:57:19 2015 -0500

    rtspconnection: Use relative URI for non-proxy tunneled requests
    
    Match the section 5.1.2 of the HTTP/1.0 spec by using relative URIs unless we
    are using a proxy server. Also, send Host header for compatability with
    HTTP/1.1 and some HTTP/1.0 servers.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=758922