Better handling for link rel dns-prefetch and preconnect#536
Better handling for link rel dns-prefetch and preconnect#536cjmayo merged 4 commits intolinkchecker:masterfrom
Conversation
linkcheck/htmlutil/linkparse.py
Outdated
| # note: value can be None | ||
| value = attrs.get(attr) | ||
| if tag == 'link' and attrs.get('rel') == 'dns-prefetch': | ||
| if tag == 'link' and ('dns-prefetch' in attrs.get('rel') or 'preconnect' in attrs.get('rel')): |
There was a problem hiding this comment.
What does attrs.get('rel') return when there's no rel attr? If None, the proposed code will crash with a TypeError.
Ideally there'd be a unit test with a <link> without rel, to see that this works.
There was a problem hiding this comment.
Also, welp, flake8 complains:
./linkcheck/htmlutil/linkparse.py:160:89: E501 line too long (106 > 88 characters)
There was a problem hiding this comment.
You're absolutely right. I added a fallback for this, and a test that makes sure no TypeError is raised (there's probably a better way, but it does the job).
|
Looks like we ran out of Travis CI credits and need to migrate to something else (GitHub Actions) before we can see whether tests pass on pull requests. sigh more work... |
|
We now have working GitHub actions. Let's see what happens if I push the 'Update branch' button GitHub shoves at me... |
bc93ce9 to
282e986
Compare
cjmayo
left a comment
There was a problem hiding this comment.
Checking DNS only is allowed even in the Resource Hints Editor's Draft
https://w3c.github.io/resource-hints/#preconnect
|
Thanks for the improvement and especially the tests. |
…536) preconnect is only DNS checked. This is allowed even in the Resource Hints Editor's Draft https://w3c.github.io/resource-hints/#preconnect 900586d
|
You are most welcome! :) Glad to see this merged. |
I changed a few things in regards to handling link tags, in particular
dns-prefetchandpreconnect.relattribute can have multiple values, separated by a space, so switched to substring comparisonpreconnectusually uses the root URL (eghttps://fonts.googleapis.com) for setting up a connection, but that URL can return a 404 in some cases, resulting in a false positive. I changed it to a DNS check for now. This could be changed in the future to use a check for a valid http connection.I added some tests, too :)