Bug 213245 – iOS Safari incorrectly reports "AppleCoreMedia" as UA string

• Component: Media
• Status: RESOLVED FIXED

[John Luther]

Safari on iOS reports an incorrect user agent string (AppleCoreMedia). This impacts us at JW Player by making it impossible to distinguish between native mobile apps and mobile web in our analytics. 

Filing this bug at the recommendation of Jer Noble.

[Alexey Proskuryakov]

Jer Noble would know better, but how is this a WebKit issue? This indicates that loading didn't go through WebKit, as it should have.

Do you have steps to reproduce that would work for us?

[Radar WebKit Bug Importer]

<rdar://problem/64471582>

[Jer Noble]

We pass a custom user-agent string to AVFoundation in a dictionary of header values, but that particular one appears to be ignored. We can work around this behavior by overriding the user-agent value at the WebCoreNSURLSession level.

[Jer Noble]

And the issue is trivially reproducible. You can verify that we're sending the wrong UA string in WebInspector with any page with a <video> element.

[Alexey Proskuryakov]

I see, so this was a misunderstanding: "loading didn't go through WebKit".

[Jer Noble]

Created attachment 402718 [details]
Patch

[youenn fablet]

Comment on attachment 402718 [details]
Patch

LGTM.
In theory, CoreMedia should probably split their code so that they only set these headers (user-agent, accept...) in the code that actually sends the requests themselves.

View in context: https://bugs.webkit.org/attachment.cgi?id=402718&action=review

> Tools/TestWebKitAPI/Tests/WebKitCocoa/MediaLoading.mm:38
> +static String parseUserAgent(Vector<char>&& request)

const Vector<char>&

> Tools/TestWebKitAPI/Tests/WebKitCocoa/MediaLoading.mm:40
> +    Vector<String> headers = String::fromUTF8(request.data(), request.size()).split("\r\n");

auto

> Tools/TestWebKitAPI/Tests/WebKitCocoa/MediaLoading.mm:41
> +    auto index = headers.findMatching([] (const String& header) { return header.startsWith("User-Agent:"); });

auto

> Tools/TestWebKitAPI/Tests/WebKitCocoa/MediaLoading.mm:57
> +        connection.receiveHTTPRequest([&] (Vector<char>&& request) {

auto&&

> Tools/TestWebKitAPI/Tests/WebKitCocoa/MediaLoading.mm:58
> +            auto userAgent = parseUserAgent(WTFMove(request));

no need to move. Ditto below.

[Jer Noble]

Created attachment 402744 [details]
Patch for landing

[EWS]

ChangeLog entry in Source/WebCore/ChangeLog contains OOPS!.

[Alex Christensen]

Comment on attachment 402744 [details]
Patch for landing

View in context: https://bugs.webkit.org/attachment.cgi?id=402744&action=review

source change looks fine

> Tools/TestWebKitAPI/Tests/WebKitCocoa/MediaLoading.mm:38
> +static String parseUserAgent(const Vector<char>& request)

Couldn't you just look for "User-Agent: TestWebKitAPI\r\n" in the request instead of this?

> Tools/TestWebKitAPI/Tests/WebKitCocoa/MediaLoading.mm:52
> +    webView.get()._customUserAgent = @"TestWebKitAPI";

Could you use customUserAgent instead of _customUserAgent?

> Tools/TestWebKitAPI/Tests/WebKitCocoa/MediaLoading.mm:105
> +            connection.send(makeString("HTTP/1.1 200 OK\r\n",

Does this test even need to send a reply?

[Jer Noble]

Created attachment 402799 [details]
Patch for landing

[Jer Noble]

Comment on attachment 402744 [details]
Patch for landing

View in context: https://bugs.webkit.org/attachment.cgi?id=402744&action=review

>> Tools/TestWebKitAPI/Tests/WebKitCocoa/MediaLoading.mm:38
>> +static String parseUserAgent(const Vector<char>& request)
> 
> Couldn't you just look for "User-Agent: TestWebKitAPI\r\n" in the request instead of this?

I could; but with this way, a failing test shows the incorrect User-Agent in the test output, rather than just a "strncmp expected TRUE, got FALSE".

>> Tools/TestWebKitAPI/Tests/WebKitCocoa/MediaLoading.mm:52
>> +    webView.get()._customUserAgent = @"TestWebKitAPI";
> 
> Could you use customUserAgent instead of _customUserAgent?

I didn't know that was a thing! Yes, it could.

>> Tools/TestWebKitAPI/Tests/WebKitCocoa/MediaLoading.mm:105
>> +            connection.send(makeString("HTTP/1.1 200 OK\r\n",
> 
> Does this test even need to send a reply?

Yes it does. I wanted to make sure both the initial request for the manifest contained the correct MIME type, and also follow-up requests for media segments also did.  Without sending a correct reply, the follow up media request will never be sent.

[Alex Christensen]

Comment on attachment 402799 [details]
Patch for landing

Aha, you're right.  I thought receivedManifestRequest and receivedMediaRequest were the same.  It turns out, they're not the same.

[Jer Noble]

Created attachment 402802 [details]
Patch for landing

[EWS]

Committed r263537: <https://trac.webkit.org/changeset/263537>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 402802 [details].

[Jonathan Bedard]

This broke tvOS and watchOS builds: https://build.webkit.org/builders/Apple-tvOS-13-Release-Build/builds/171

(we don't have EWS yet, operations has been consumed by Big Sur work, it's coming soon)

[Yusuke Suzuki]

Re-opened since this is blocked by bug 213640

[Jer Noble]

Okay, looks like tvOS and watchOS don't `HAVE(NETWORK_FRAMEWORK)`. Just needs a compile guard around the test case.

[Jer Noble]

Created attachment 402926 [details]
Patch for landing

[EWS]

Committed r263627: <https://trac.webkit.org/changeset/263627>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 402926 [details].