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.

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

Query parameter falsifies the image id #205

Open
Ayax0 opened this issue Jan 6, 2024 · 2 comments
Open

Query parameter falsifies the image id #205

Ayax0 opened this issue Jan 6, 2024 · 2 comments

Comments

@Ayax0
Copy link

Ayax0 commented Jan 6, 2024

Environment

Node: 21.5.0
ipx: 2.0.2

Reproduction

I have built my own image provider with NuxtImage which provides the images via an API route. The reason for this procedure is that I add the access_token as a query parameter to the image url so that the images can only be accessed with authentication. If I now create a normal IPXH3 handler, I get the error message:

I have solved the problem by copying the createIPXH3Handler function from "/src/server.ts" and changing line 28 event.path to event.path.split("?")[0]. I don't know if it would be possible to add this in general or if there is a better solution.

Describe the bug

Query parameters are included in the image id

Additional context

No response

Logs

{"error":{"message":"[403] [IPX_FORBIDDEN_PATH] Forbidden path: /TestImage.png?test=123"}}
@pi0
Copy link
Member

pi0 commented Jan 11, 2024

Hi dear @Ayax0. Thanks for explaining your usecase.

From a quick look, I would say it probably worth to move authentication headers out of query parameters to the headers. query params are an unsafe place for it and can cause cache leak issues. (if you can share your project or a reproduction similar to it I can take a look)

Also omiting ? part of URL, can cause so many side cases that actually benefit from it being as part of source URL. So this is not a fix we can just make by default (but we can add behind an option, if makes sense)

As a simpler workaround you can make a wrapper handler like this:

const handler = eventHandler((event) => {
  event.path = event.path.split('?')[0]
  return ipxHandler(event)
})

@Ayax0
Copy link
Author

Ayax0 commented Jan 14, 2024

Hi @pi0
first of all, thank you for your great work in the whole nuxt / unjs ecosystem.
I have now tested a little further and have moved the authentication to the header as explained here:
nuxt/image#984 (comment)

As you mentioned it might actually make more sense to make this function optional.

Thanks for your answer!

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 a pull request may close this issue.

2 participants