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.

Sign up for GitHub

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

Jump to bottom

Don't check preconnect links if not in verbatim mode #897

Open
mre opened this issue Jan 3, 2023 · 4 comments · May be fixed by #1187
Open

Don't check preconnect links if not in verbatim mode #897

mre opened this issue Jan 3, 2023 · 4 comments · May be fixed by #1187
Labels
bug Something isn't working enhancement New feature or request false-positive

Comments

@mre
Copy link
Member

mre commented Jan 3, 2023

Preconnect links are used to establish a server connection without loading a specific resource yet.
Not always do these links point to a URL that should return a 200 and they are not user-facing, i.e. they don't show up in the final rendered version of a page.
Therefore I think we should only check them if --include-verbatim is enabled.

Examples from https://www.algolia.com/search/:

<link rel="preconnect" href="https://1QDAWL72TQ-dsn.algolia.net" />
<link rel="preconnect" href="https://cdn.cookielaw.org" />
<link rel="preconnect" href="https://res.cloudinary.com" />

Reference: #895.
@mre mre added enhancement New feature or request false-positive labels Jan 3, 2023
@lebensterben
Copy link
Member

lebensterben commented Jan 4, 2023 •
edited

We might as well deal with other pre* links all together.

https://html.spec.whatwg.org/#linkTypes

@mre
Copy link
Member Author

mre commented Jan 4, 2023

Yeah I thought about that, however the other pre-links should all be valid links as far as I can tell:

  • preconnect is more like a DNS lookup + TCP handshake as far as I understand.
  • prefetch, preload, and prerender on the other side fetch the resource, so the link has to be valid to do so.

@lebensterben
Copy link
Member

preconnect:

Specifies that the user agent should preemptively connect to the target resource's origin.

where origin is

A tuple consists of:

A scheme (an ASCII string).
A host (a host).
A port (null or a 16-bit unsigned integer).
A domain (null or a domain). Null unless stated otherwise.

Following the same logic, we should also validate the origin then...

@mre mre added the bug Something isn't working label Jul 28, 2023
@mre mre linked a pull request Jul 29, 2023 that will close this issue
Open
mre added a commit that referenced this issue Jul 29, 2023
@mre
Don't check preconnect links
dc397f6 
Preconnect links are used to establish a server connection without loading a
specific resource yet. Not always do these links point to a URL that should
return a 200, and they are not user-facing, i.e. they don't show up in the
final rendered version of a page.

Therefore, I think we should them at all; not even in `--include-verbatim`
mode, as they might not point to a valid resource.

Fixes #897
mre added a commit that referenced this issue Jul 29, 2023
@mre
Don't check preconnect links
3d10611 
Preconnect links are used to establish a server connection without loading a
specific resource yet. Not always do these links point to a URL that should
return a 200, and they are not user-facing, i.e. they don't show up in the
final rendered version of a page.

Therefore, I think we should them at all; not even in `--include-verbatim`
mode, as they might not point to a valid resource.

Fixes #897
@mre
Copy link
Member Author

mre commented Jul 29, 2023

The preconnect directive is primarily about establishing a network connection to the specified domain in advance of any actual request. The fact that a request to e.g. the root of https://1QDAWL72TQ-dsn.algolia.net returns a 404 doesn't necessarily mean that the preconnect directive is misconfigured or ineffective.

When a browser sees a preconnect directive, it's not immediately making a GET request to that URL. Instead, it's only performing the lower-level networking setup (i.e. DNS resolution, establishing a TCP connection, and performing a TLS handshake if the connection is secure). This is all done in anticipation of a request being made to that domain later.

If a later request is made to a specific path on that domain (for example, https://1QDAWL72TQ-dsn.algolia.net/some/resource/path), the fact that the root of the domain (https://1QDAWL72TQ-dsn.algolia.net/) returns a 404 is irrelevant. What matters is whether the specific resource requested returns a valid response.

Hence, I think we should exclude preconnect directives.

PR for that: #1187

mre added a commit that referenced this issue Aug 15, 2023
@mre
Don't check preconnect links
0732b09 
Preconnect links are used to establish a server connection without loading a
specific resource yet. Not always do these links point to a URL that should
return a 200, and they are not user-facing, i.e. they don't show up in the
final rendered version of a page.

Therefore, I think we should them at all; not even in `--include-verbatim`
mode, as they might not point to a valid resource.

Fixes #897
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working enhancement New feature or request false-positive
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Don't check preconnect links lycheeverse/lychee
2 participants
@mre @lebensterben