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 774768 - rtsp: duplicated content-length error.
rtsp: duplicated content-length error.
Status: RESOLVED NOTABUG
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
unspecified
Other Linux
: Normal major
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-11-21 04:01 UTC by Sangkyu Park (sangkyup)
Modified: 2016-11-21 11:22 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
a patch for this bug (973 bytes, application/mbox)
2016-11-21 04:01 UTC, Sangkyu Park (sangkyup)
  Details
a patch for this bug. (973 bytes, patch)
2016-11-21 04:13 UTC, Sangkyu Park (sangkyup)
rejected Details | Review

Description Sangkyu Park (sangkyup) 2016-11-21 04:01:11 UTC
Created attachment 340386 [details]
a patch for this bug

There is duplicated rtsp content-length error.
The specific deivce disconnects rtsp connection due to this bug.
So I checked tcpdump and found this bug.
If you add content-length header to message using "gst_rtsp_message_add_header", the content length is duplicated twice.
You can check it in tcpdump rtsp text.

for example,
RTSP/1.0 200 OK
CSeq: 2
Content-Type: text/parameters
Content-Length: 645
Date: Thu, 01 Jan 1970 00:02:27 GMT
Content-Length: 645
Comment 1 Sangkyu Park (sangkyup) 2016-11-21 04:13:27 UTC
Created attachment 340388 [details] [review]
a patch for this bug.

update the patch.
Comment 2 Sebastian Dröge (slomo) 2016-11-21 07:42:07 UTC
Where is it added the first time? How can this be reproduced?
Comment 3 Sangkyu Park (sangkyup) 2016-11-21 07:56:54 UTC
in message_to_string 

1) first
    /* append headers */
    gst_rtsp_message_append_headers (message, str);

2) second
    len = g_strdup_printf ("%d", message->body_size);
    g_string_append_printf (str, "%s: %s\r\n",
        gst_rtsp_header_as_text (GST_RTSP_HDR_CONTENT_LENGTH), len);

I think, first time is not necessary. So I removed the content-length in message.
You can reproduce as following :

1. set body
2. set content length
3. send the rtsp message

Capture tcpdump for the packet and check.
you can check the duplicated issue in real rtsp text
Comment 4 Sebastian Dröge (slomo) 2016-11-21 08:22:19 UTC
Who was originally setting the content length though, if GstRTSPConnection always sets it by itself anyway?
Comment 5 Sangkyu Park (sangkyup) 2016-11-21 09:03:55 UTC
I can not answer your question exactly.
I think..if gstrtspconnection sets the length using body length,
it can handle wrong conten length that is set from user.

The gstreamer developers can set the content length as below.
gst_rtsp_message_add_header (msg, GST_RTSP_HDR_CONTENT_LENGTH, legnth);
Comment 6 Sebastian Dröge (slomo) 2016-11-21 09:11:39 UTC
Yes but who does set it manually in your case? That code should be fixed as the connection will already do that.
Comment 7 Sangkyu Park (sangkyup) 2016-11-21 10:04:15 UTC
Yes We've set the content length in manually. I will also remove to set the length because it's unnecessary.
Comment 8 Sebastian Dröge (slomo) 2016-11-21 10:14:15 UTC
Which code does that?
Comment 9 Sangkyu Park (sangkyup) 2016-11-21 10:16:41 UTC
It is our custom rtsp src plugin using rtspconnection.
Comment 10 Sangkyu Park (sangkyup) 2016-11-21 10:27:55 UTC
In current status, Any person using gstrtsp sets rtsp body and content-length in menually.
The length is duplicated in real packet..
Comment 11 Sebastian Dröge (slomo) 2016-11-21 10:28:12 UTC
I see, that's a bug in your code then :)
Comment 12 Sangkyu Park (sangkyup) 2016-11-21 10:32:49 UTC
The patch is rejected.. Is it wrong ?
Comment 13 Sangkyu Park (sangkyup) 2016-11-21 10:47:12 UTC
Hm.. I think the content length should be set once in real packet, even though we set the length in menually.
Comment 14 Sebastian Dröge (slomo) 2016-11-21 10:57:38 UTC
The bug is in your code. You're not supposed to add the Content-Length header yourself but GstRTSPConnection is doing that.
Comment 15 Sebastian Dröge (slomo) 2016-11-21 11:00:39 UTC
Or am I misunderstanding what you mean?
Comment 16 Sangkyu Park (sangkyup) 2016-11-21 11:02:58 UTC
I can not expect to set content-Length by gstrtspconnection in automatically. Thank you for response.
Comment 17 Sebastian Dröge (slomo) 2016-11-21 11:05:47 UTC
Why can't you expect GstRTSPConnection to do that?
Comment 18 Sangkyu Park (sangkyup) 2016-11-21 11:09:56 UTC
We add many fields to rtsp header such as
GST_RTSP_HDR_PUBLIC, GST_RTSP_HDR_USER_AGENT, GST_RTSP_HDR_CONTENT_TYPE, GST_RTSP_HDR_REQUIRE and etc..
So we set GST_RTSP_HDR_CONTENT_LENGTH also. We can not expect that duplicated in real packet.
Comment 19 Sebastian Dröge (slomo) 2016-11-21 11:18:37 UTC
You mean the documentation should make clear that you must not set Content-Length because the connection already does that for you?

The connection is also setting various other headers for you already, especially for authentication.
Comment 20 Sangkyu Park (sangkyup) 2016-11-21 11:22:55 UTC
Yes. It needs some guide or handle the exception case in automatically.
In my just opinion, second way will be better. So I made the patch..