Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add content-type to resource-timing #341

Merged
merged 9 commits into from
May 11, 2023
Merged

Conversation

abinpaul1
Copy link
Contributor

@abinpaul1 abinpaul1 commented Aug 22, 2022

Introducing Content type. (#203)
Fetch changes : whatwg/fetch#1481

Explainer : https://github.com/abinpaul1/resource-timing/blob/explainer-content-type/Explainers/Content-Type.md


💥 Error: Server Error 💥

PR Preview failed to build. (Last tried on May 8, 2023, 11:28 AM UTC).

More

If you don't have enough information above to solve the error by yourself (or to understand to which web service the error is related to, if any), please file an issue.

@abinpaul1 abinpaul1 marked this pull request as draft August 22, 2022 15:02
@abinpaul1
Copy link
Contributor Author

abinpaul1 commented Sep 4, 2022

I was trying to spec this out keeping in mind, the points mentioning not to reflect the header directly and only a set of relevant MIME types.

  • It is currently(in this PR) specified to allow any MIME type that matches the groups presented in https://mimesniff.spec.whatwg.org/#mime-type-groups.
  • Taking the case of say a ZIP-based MIME type, its described as any MIME type whose subtype ends in "+zip". So a content type such as application/u.26363-43323-42424+zip would be valid and reflected, which is not desired.
  • To get around this, AFAIU we would need to define an exhaustive list of allowed MIME types to match against. I don't think we can go with the list that was mentioned in the WG call (https://www.iana.org/assignments/media-types/media-types.xhtml) because a good majority of those do not seem to be relevant for the use cases.
  • If we were to keep an exhaustive list, which would be the more appropriate place to keep this? (MIME sniffing/Resource Timing). Such a list would have to updated as well when a new relevant content-type gains traction as well.

@yoavweiss
Copy link
Contributor

@abinpaul1 - can we just reflect the MIME type's essensce?

@abinpaul1
Copy link
Contributor Author

Yeah, but when we reflect the essence wouldn't this be a valid essence : application/u.26363-43323-42424+zip ?
Here application would be type and u.26363-43323-42424+zip is the subtype

@yoavweiss
Copy link
Contributor

Yeah, you're right.. Maybe it'd be possible to convince the MIMESNIFF editors to modify that definition, or otherwise monkeypatch it, if ZIP is the only mimetype that can expose such UIDs

/cc @domenic @annevk

@abinpaul1
Copy link
Contributor Author

abinpaul1 commented Sep 5, 2022

I don't think ZIP is the only one that can have such UIDs.
JSON, XML MIME types also only requires subtype to end with +json, +xml. Morover, Image and audio/video MIME types does not have any restrictions on subtype (only requires the type to be image/audio/video)

@domenic
Copy link

domenic commented Sep 5, 2022

Why is the full MIME type string not reflected? I couldn't find any reasoning in the explainer at https://github.com/abinpaul1/resource-timing/blob/explainer-content-type/Explainers/Content-Type.md#content-type-values.

@abinpaul1
Copy link
Contributor Author

abinpaul1 commented Sep 5, 2022

During the WG call when this was discussed, a concern was raised regarding reflecting the exact same value returned by server.
From what I could understand the concern was mainly to protect against having any unique identifier as part of content-type in server response being relflected through resource timing API. @achristensen07, it would be great if you could please confirm or expand on the concern.

@yoavweiss
Copy link
Contributor

I believe the concerns are related to w3c/server-timing#89 and are planned to be discussed at TPAC.

@abinpaul1 abinpaul1 marked this pull request as ready for review September 26, 2022 15:43
Copy link
Contributor

@yoavweiss yoavweiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM once the Fetch PR lands

@abinpaul1 abinpaul1 changed the title [draft] Add content-type to resource-timing Add content-type to resource-timing Sep 26, 2022
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Oct 24, 2022
This CL introduces a contentType field to Performance Resource
Timing object. This field is behind a Runtime Enabled Flag.

Resource Timing PR : w3c/resource-timing#341
Fetch PR : whatwg/fetch#1481

Bug:1366706
Change-Id: If4c92ef96d74e3ddbb71420ac61dbea4bfa1163b
aarongable pushed a commit to chromium/chromium that referenced this pull request Oct 25, 2022
This CL introduces a contentType field to Performance Resource
Timing object. This field is behind a Runtime Enabled Flag.

Resource Timing PR : w3c/resource-timing#341
Fetch PR : whatwg/fetch#1481

Bug: 1366706
Change-Id: If4c92ef96d74e3ddbb71420ac61dbea4bfa1163b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3916841
Reviewed-by: Yoav Weiss <[email protected]>
Reviewed-by: Mike West <[email protected]>
Commit-Queue: Abin Paul <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1063289}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Oct 25, 2022
This CL introduces a contentType field to Performance Resource
Timing object. This field is behind a Runtime Enabled Flag.

Resource Timing PR : w3c/resource-timing#341
Fetch PR : whatwg/fetch#1481

Bug: 1366706
Change-Id: If4c92ef96d74e3ddbb71420ac61dbea4bfa1163b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3916841
Reviewed-by: Yoav Weiss <[email protected]>
Reviewed-by: Mike West <[email protected]>
Commit-Queue: Abin Paul <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1063289}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Oct 25, 2022
This CL introduces a contentType field to Performance Resource
Timing object. This field is behind a Runtime Enabled Flag.

Resource Timing PR : w3c/resource-timing#341
Fetch PR : whatwg/fetch#1481

Bug: 1366706
Change-Id: If4c92ef96d74e3ddbb71420ac61dbea4bfa1163b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3916841
Reviewed-by: Yoav Weiss <[email protected]>
Reviewed-by: Mike West <[email protected]>
Commit-Queue: Abin Paul <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1063289}
Copy link
Contributor

@yoavweiss yoavweiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Nov 12, 2022
…iming, a=testonly

Automatic update from web-platform-tests
Add content-type to PerformanceResourceTiming

This CL introduces a contentType field to Performance Resource
Timing object. This field is behind a Runtime Enabled Flag.

Resource Timing PR : w3c/resource-timing#341
Fetch PR : whatwg/fetch#1481

Bug: 1366706
Change-Id: If4c92ef96d74e3ddbb71420ac61dbea4bfa1163b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3916841
Reviewed-by: Yoav Weiss <[email protected]>
Reviewed-by: Mike West <[email protected]>
Commit-Queue: Abin Paul <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1063289}

--

wpt-commits: 4c8e480994182b5cdc60f5c01ebf208df70ec73e
wpt-pr: 36059
jamienicol pushed a commit to jamienicol/gecko that referenced this pull request Nov 14, 2022
…iming, a=testonly

Automatic update from web-platform-tests
Add content-type to PerformanceResourceTiming

This CL introduces a contentType field to Performance Resource
Timing object. This field is behind a Runtime Enabled Flag.

Resource Timing PR : w3c/resource-timing#341
Fetch PR : whatwg/fetch#1481

Bug: 1366706
Change-Id: If4c92ef96d74e3ddbb71420ac61dbea4bfa1163b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3916841
Reviewed-by: Yoav Weiss <[email protected]>
Reviewed-by: Mike West <[email protected]>
Commit-Queue: Abin Paul <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1063289}

--

wpt-commits: 4c8e480994182b5cdc60f5c01ebf208df70ec73e
wpt-pr: 36059
@RByers
Copy link

RByers commented Apr 25, 2023

Is there something blocking this PR from landing?
Context: blink intent to ship

@yoavweiss
Copy link
Contributor

This is waiting on the Fetch PR to land.

annevk pushed a commit to abinpaul1/fetch that referenced this pull request May 4, 2023
This defines the content type field as a minimized version of the response's parsed Content-Type header, to give web developers enough information to distinguish responses, but not enough to make it an additional communication channel.

This feature will be used by Resource Timing as per w3c/resource-timing#341.

Tests: ...
annevk added a commit to whatwg/fetch that referenced this pull request May 8, 2023
This defines the content type field as a minimized version of the response's parsed Content-Type header, to give web developers enough information to distinguish responses, but not enough to make it an additional communication channel.

This feature will be used by Resource Timing as per w3c/resource-timing#341.

Co-authored-by: Anne van Kesteren <[email protected]>
Copy link
Contributor

@yoavweiss yoavweiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@yoavweiss yoavweiss merged commit 41d9808 into w3c:gh-pages May 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants