Skip to content

feat: validate external domains - #343

Merged
pi0 merged 7 commits into
mainfrom
fix/vercel-validate-domains
Jun 29, 2021
Merged

feat: validate external domains#343
pi0 merged 7 commits into
mainfrom
fix/vercel-validate-domains

Conversation

@farnabaz

Copy link
Copy Markdown
Member

Add logic to serve original src if image does not serve from listed domains

Resolves #342

@codecov-commenter

codecov-commenter commented Jun 28, 2021

Copy link
Copy Markdown

Codecov Report

Merging #343 (f117bd7) into main (4711fee) will decrease coverage by 0.06%.
The diff coverage is 44.44%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #343      +/-   ##
==========================================
- Coverage   59.36%   59.30%   -0.07%     
==========================================
  Files          26       26              
  Lines         598      602       +4     
  Branches      188      188              
==========================================
+ Hits          355      357       +2     
- Misses        243      245       +2     
Impacted Files Coverage Δ
src/provider.ts 74.28% <ø> (+2.06%) ⬆️
src/runtime/providers/vercel.ts 0.00% <0.00%> (ø)
src/runtime/image.ts 66.37% <25.00%> (-1.52%) ⬇️
src/module.ts 92.85% <50.00%> (-3.30%) ⬇️
src/runtime/providers/ipx.ts 100.00% <100.00%> (+15.38%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4711fee...f117bd7. Read the comment docs.

@pi0

pi0 commented Jun 28, 2021

Copy link
Copy Markdown
Member

Thanks for pull-request @farnabaz. I think we shall move this validation out of provider implementation and always validate to externalize URLs when domains option is provided and current URL is not matching.

@atinux

atinux commented Jun 28, 2021

Copy link
Copy Markdown
Member

On which file/function can he implement the validation @pi0 ?

@pi0

pi0 commented Jun 28, 2021

Copy link
Copy Markdown
Member

I think here we can add the hostname check:

@farnabaz

Copy link
Copy Markdown
Member Author

I moved domain validation into resolveImage helper and removed logic from both ipx and vercel providers.

Does it requires other changes @pi0?

@pi0

pi0 commented Jun 29, 2021

Copy link
Copy Markdown
Member

Changes look good @farnabaz 👌 I'm adding a change to ipx using hostname to use unified method and avoid startsWith check for vercel

pi0 added a commit to unjs/ipx that referenced this pull request Jun 29, 2021
@pi0

pi0 commented Jun 29, 2021

Copy link
Copy Markdown
Member
  • Updated ipx with hostname check
  • Domains are normalized in module
  • Only do external check if provider exports validateDomains flag enabled for ipx and vercel. This is to avoid breaking prismic and storyblok as they accept full URLs (will probably add a flag for this incompatible providers instead later for now flag is undocumented)

@pi0 pi0 changed the title fix(vercel): validate remote domains feat: validate external domains Jun 29, 2021
@pi0
pi0 merged commit bee0040 into main Jun 29, 2021
@pi0
pi0 deleted the fix/vercel-validate-domains branch June 29, 2021 16:38
procrates pushed a commit to procrates/nuxt-image that referenced this pull request Feb 21, 2023
Co-authored-by: pooya parsa <pyapar@gmail.com>
Polaris-dev9 added a commit to Polaris-dev9/TypeScript-ipx that referenced this pull request May 1, 2025
@github-actions github-actions Bot mentioned this pull request Jul 30, 2026
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.

Vercel Provider does not respect domains list

4 participants