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.
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
feat: support subdomains in isIPFS.url(url) #32
feat: support subdomains in isIPFS.url(url) #32
Changes from 4 commits
cc40cd4
2644db0
72c0344
f1823cc
c520efc
d9e7082
d5717e9
87d746a
404957e
18d80d4
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Is this a thing now?
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.
Yes,
?filename=
sets proper Content-Disposition header and all that jazz. Folks use it as more efficient way of sharing single file, as it keeps the same CID while enables filename tweaks (does not require wrapping in a directory)I've added it to README as its important to show that URL params do not impact CID validation.
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.
I know it does that for URLs on the gateways but I didn't know if we'd formalised it in IPFS paths.
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.
Ah, I get your point now.
No, it is not formalized in the context of IPFS paths –
?filename=foo.jpg
is just a valid filename in IPFS.See this example:
When exposed on the gateway, filenames are URIencoded:
https://ipfs.io/ipfs/QmbcBPAwCDxRMB1Qe7CRQmxdrTSkxKwM9y6rZw2FjGtbsb/%3Ffilename=test.jpg
Here I just forgot
/
before?
, but I agree the naming overlap is confusing, so I changed it to be something else: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.
Does it check for a case insensitive encoding or is this actually just base32?
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.
Web browsers force-lowercase hostname in URL and is-ipfs does the same before we check if CID in subdomain is valid:
https://github.com/ipfs/is-ipfs/blob/f1823cc0b12e97ac1f202ba7c06663b5b9b5d265/src/index.js#L83-L87