1026923 - [RTSP] Change the User-Agent string of RTSP client

Status: RESOLVED FIXED

(Firefox OS Graveyard :: RTSP, defect)

Description

[Ethan Tseng [:ethan]]

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.

Comment 1

[Ethan Tseng [:ethan]]

There was a bug for user story: Bug 940549 - [User Story] [RTSP] UA for RTSP.

Comment 2

[Ethan Tseng [:ethan]]

Attachment: rtsp-ua-string.patch

Change the value of User Agent in RTSP requests.

Comment 3

[Ethan Tseng [:ethan]]

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

[Steve Workman [:sworkman] (INACTIVE)]

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.

Comment 5

[Ethan Tseng [:ethan]]

Attachment: Change User-Agent in RTSP

Use nsIHttpProtocolHandler to get User-Agent.

Comment 6

[Ethan Tseng [:ethan]]

(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

[Steve Workman [:sworkman] (INACTIVE)]

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! :)

Comment 8

[Ethan Tseng [:ethan]]

(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.

Comment 9

[Ethan Tseng [:ethan]]

Attachment: Change User-Agent in RTSP

Refresh comment "r=sworkman".

Comment 10

[Ethan Tseng [:ethan]]

The result of Try server:
https://tbpl.mozilla.org/?tree=Try&rev=ff72269489f2

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/917fbaf92fa2

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/917fbaf92fa2