Closed
Bug 1026923
Opened 10 years ago
Closed 10 years ago
[RTSP] Change the User-Agent string of RTSP client
Categories
(Firefox OS Graveyard :: RTSP, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
2.0 S6 (18july)
People
(Reporter: ethan, Assigned: ethan)
Details
(Whiteboard: [p=2])
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
Details | Diff | Splinter Review |
Our RTSP client was ported from Android's libstagefright, which specifies the user-agent string in RTSP request headers in the following format:
"stagefright/1.1 (Linux;Android <ro.build.version.release>)"
For example, on Flame 2.0, the user-agent string in RTSP request headers is:
"stagefright/1.1 (Linux;Android 4.3)"
We should change the user-agent string to reflect the application is on Firefox OS device.
Assignee | ||
Comment 1•10 years ago
|
||
There was a bug for user story: Bug 940549 - [User Story] [RTSP] UA for RTSP.
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → ettseng
Assignee | ||
Updated•10 years ago
|
Status: NEW → ASSIGNED
Whiteboard: [p=2]
Target Milestone: --- → 2.0 S6 (18july)
Assignee | ||
Comment 2•10 years ago
|
||
Change the value of User Agent in RTSP requests.
Attachment #8457913 -
Flags: review?(sworkman)
Assignee | ||
Comment 3•10 years ago
|
||
Hi Steve,
Could you help to review this patch?
I decided to generate the value of User-Agent in RtspController and pass it to RTSPSource/RTSPConnectionHandler/ARTSPConnection.
It's because the codes under netwerk/protocol/rtsp/rtsp/ are more independent from Gecko and I don't want to add much binding to B2G there.
The new User-Agent would be like this:
User-Agent: Mozilla (Firefox OS 2.1.0.0-prerelease; Gecko 33.0a1; Build 20140717175805)\r\n
Comment 4•10 years ago
|
||
Comment on attachment 8457913 [details] [diff] [review]
rtsp-ua-string.patch
Review of attachment 8457913 [details] [diff] [review]:
-----------------------------------------------------------------
So, I'm not a UA expert, but is this definitely something you want to have different from the HTTP UA string? It seems like you could make it easier by just using whatever UA is already generated for the rest of the OS, and have it match what is sent to HTTP servers. No?
::: netwerk/protocol/rtsp/rtsp/RTSPConnectionHandler.h
@@ +54,5 @@
>
> namespace android {
>
> +static void MakeUserAgentString(const char* userAgent, AString *s) {
> + s->setTo(userAgent);
I think you can just get rid of this function and call s->setTo directly.
Assignee | ||
Comment 5•10 years ago
|
||
Use nsIHttpProtocolHandler to get User-Agent.
Attachment #8457913 -
Attachment is obsolete: true
Attachment #8457913 -
Flags: review?(sworkman)
Attachment #8458495 -
Flags: review?(sworkman)
Assignee | ||
Comment 6•10 years ago
|
||
(In reply to Steve Workman [:sworkman] from comment #4)
> Comment on attachment 8457913 [details] [diff] [review]
> So, I'm not a UA expert, but is this definitely something you want to have
> different from the HTTP UA string? It seems like you could make it easier by
> just using whatever UA is already generated for the rest of the OS, and have
> it match what is sent to HTTP servers. No?
You are right. I should simply use the existing UA.
RTSP spec doesn't specify anything special for User-Agent. It's better to use the same value as our browser.
Now, the UA value would be:
User-Agent: Mozilla/5.0 (Mobile; rv:33.0) Gecko/33.0 Firefox/33.0\r\n
Comment 7•10 years ago
|
||
Comment on attachment 8458495 [details] [diff] [review]
Change User-Agent in RTSP
Review of attachment 8458495 [details] [diff] [review]:
-----------------------------------------------------------------
Looks good to me! Thanks for making the changes! r=me.
::: netwerk/protocol/rtsp/controller/RtspController.cpp
@@ +371,5 @@
> + // Get User-Agent.
> + nsCOMPtr<nsIHttpProtocolHandler>
> + service(do_GetService(NS_NETWORK_PROTOCOL_CONTRACTID_PREFIX "http", &rv));
> + rv = service->GetUserAgent(mUserAgent);
> + if (NS_FAILED(rv)) return rv;
if (NS_WARN_IF(NS_FAILED(rv)) {
return rv;
}
And getting UA from HTTP looks good :) I think this will save you from problems in the future! :)
Attachment #8458495 -
Flags: review?(sworkman) → review+
Assignee | ||
Comment 8•10 years ago
|
||
(In reply to Steve Workman [:sworkman] from comment #7)
> Looks good to me! Thanks for making the changes! r=me.
> And getting UA from HTTP looks good :) I think this will save you from
> problems in the future! :)
Cool!!
Thanks for your review and advice, Steve.
Assignee | ||
Comment 9•10 years ago
|
||
Refresh comment "r=sworkman".
Attachment #8458495 -
Attachment is obsolete: true
Assignee | ||
Comment 10•10 years ago
|
||
The result of Try server:
https://tbpl.mozilla.org/?tree=Try&rev=ff72269489f2
Keywords: checkin-needed
Comment 11•10 years ago
|
||
Keywords: checkin-needed
Comment 12•10 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•