GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — http,http2: support more attributes for early hint link by anonrig · Pull Request #44874 · nodejs/node

Conversation

@anonrig

@anonrig anonrig commented Oct 3, 2022

Copy link
Copy Markdown
Member

Link header should eventually have to support all attributes of the HTML link element.

  • The list of all available attributes is available https://developer.mozilla.org/en-US/docs/Web/HTML/Element/link#attributes.
  • The syntax for the link header is available https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Link#syntax.

Resolves #44843.
Dependent of #44820.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
bricss reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Oct 3, 2022
@anonrig anonrig mentioned this pull request Oct 3, 2022
http,http2: make early hints generic #44820
Merged
@zekth

zekth commented Oct 3, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

Links in your issue are invalid
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
zekth
zekth reviewed Oct 3, 2022
View reviewed changes
Comment thread lib/internal/validators.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
@anonrig anonrig changed the title lib: support more attributes for early hint link http,http2: support more attributes for early hint link Oct 3, 2022
@ZYSzys

ZYSzys commented Oct 7, 2022

Copy link
Copy Markdown
Member

Can we add some tests ?
anonrig reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@anonrig
lib: support more attributes for early hint link
08bec68
@anonrig
anonrig force-pushed the feat/link-attributes branch from 4125c28 to 08bec68 Compare October 7, 2022 11:22
@anonrig

anonrig commented Oct 7, 2022

Copy link
Copy Markdown
Member Author

I added some tests. Thank you for the review @ZYSzys
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@anonrig
anonrig requested a review from zekth October 9, 2022 18:43
@mcollina mcollina added http Issues and PRs related to the http subsystem. http2 Issues and PRs related to the http2 subsystem. request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. labels Oct 9, 2022
mcollina
mcollina approved these changes Oct 9, 2022
View reviewed changes

@mcollina mcollina left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

lgtm

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Oct 9, 2022
@nodejs-github-bot

nodejs-github-bot commented Oct 9, 2022

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/47139/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
jasnell
jasnell approved these changes Oct 10, 2022
View reviewed changes
@zekth

zekth commented Oct 10, 2022

Copy link
Copy Markdown

anonrig requested a review from zekth 22 hours ago

I'm sorry i don't have permissions on this repo. But LGTM.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@mcollina mcollina added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Oct 11, 2022
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Oct 11, 2022
@nodejs-github-bot

nodejs-github-bot commented Oct 11, 2022

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/47176/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@anonrig

anonrig commented Oct 14, 2022

Copy link
Copy Markdown
Member Author

Can we get this through the finish line? @jasnell @mcollina
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@anonrig anonrig added the commit-queue PRs queued for automated landing through the Commit Queue. label Oct 14, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Oct 14, 2022
@nodejs-github-bot
nodejs-github-bot merged commit 2649aab into nodejs:main Oct 14, 2022
@nodejs-github-bot

nodejs-github-bot commented Oct 14, 2022

Copy link
Copy Markdown
Collaborator

Landed in 2649aab
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
This was referenced Oct 17, 2022
feat(writeEarlyHints): handle object-format early hints h3js/h3#202
Merged
The argument 'hints' must be an array or string of format - after upgrading to rc12 - worked on rc11 nuxt/nuxt#15191
Closed
RafaelGSS pushed a commit that referenced this pull request Nov 1, 2022
@anonrig @RafaelGSS
lib: support more attributes for early hint link
126d300 
PR-URL: #44874
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@RafaelGSS RafaelGSS mentioned this pull request Nov 1, 2022
v19.1.0 proposal #45269
Merged
RafaelGSS pushed a commit that referenced this pull request Nov 10, 2022
@anonrig @RafaelGSS
lib: support more attributes for early hint link
8199841 
PR-URL: #44874
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@danielleadams danielleadams mentioned this pull request Dec 30, 2022
v18.13.0 release proposal #46025
Merged
danielleadams pushed a commit that referenced this pull request Dec 30, 2022
@anonrig @danielleadams
lib: support more attributes for early hint link
2375176 
PR-URL: #44874
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
danielleadams pushed a commit that referenced this pull request Jan 3, 2023
@anonrig @danielleadams
lib: support more attributes for early hint link
f412834 
PR-URL: #44874
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@bnoordhuis bnoordhuis mentioned this pull request Feb 1, 2023
Overzealous link header validation in writeEarlyHints #46453
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mcollina mcollina mcollina approved these changes
@jasnell jasnell jasnell approved these changes
@zekth zekth Awaiting requested review from zekth

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

http Issues and PRs related to the http subsystem. http2 Issues and PRs related to the http2 subsystem. needs-ci PRs that need a full CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

response.writeEarlyHints should support all link properties

Uh oh!

There was an error while loading. Please reload this page.

6 participants

@anonrig @zekth @ZYSzys @nodejs-github-bot @mcollina @jasnell
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.