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

Using / in query params for actions with AWS Lambda function URLs #7098

Closed
CalebBassham opened this issue Sep 30, 2022 · 9 comments
Closed

Comments

@CalebBassham
Copy link

CalebBassham commented Sep 30, 2022

Describe the problem

SvelteKit uses / as a signal character in query parameter keys. AWS Lambda Function URLs will not handle requests with a / in the query params. They just respond with a 400. I have opened a support ticket with them as well and they say the reasoning is that according to section 3.4 in this rfc https://www.ietf.org/rfc/rfc2396.txt, / is a reserved character.

3.4. Query Component

The query component is a string of information to be interpreted by
the resource.

  query         = *uric

Within a query component, the characters ";", "/", "?", ":", "@",
"&", "=", "+", ",", and "$" are reserved.

I have let them know that many other services allow it and handle it just fine. Including some other AWS services like CloudFront. So now the ticket is being "escalated" to the lambda team. Probably escalated to the bottom of their backlog.

Describe the proposed solution

In the SvelteKit config you could designate a different character. But to not break using other adapters it would have to be some sort of middle man fix.

Ideally, I would still want to use a / in my form action to not break compatibility with other adapters.

<form method='post' action='?/someAction'>
</form>

But, I guess this would be in the compilation phase, SvelteKit would transform those to the alternative signal character. SvelteKit would also be looking for the alternative signal character on the server.

Okay, this sucks too, but I can't think of a better way either. Maybe alternative 4.

Alternatives considered

  1. Change the signal character in SvelteKit to no longer be a / and instead be something that is not reserved. I don't necessarily think this is a great option though as everybody has already migrated and is using / and realistically this is an edge case. / makes for pretty natural looking urls when using actions which is a nice bonus.

  2. The solution proposed by AWS is that I can url encode the / every time (%2F I believe). This sucks because then I have to go to all my forms and do this ugly mess

<form method='post' action='?%2FsomeAction'>
</form>

And then make sure in my adapter to decode it before passing to SvelteKit. This is terrible for multiple reasons. The first being looking at that makes me want to puke and also would break using any other adapter.

  1. Of course I could basically implement the proposed solution of using a different signal character just in an adapter. Basically same steps of alternative 2, but instead of decoding %2F to / before SvelteKit serves the request, translate # to / or something similar. I don't know if that is even valid, but just and example. This maybe sucks less than having to write %2F all over my code base, but I am still breaking compatibility with other adapters which sucks.

  2. Just thought of this one. There could be a SvelteKit config option to, in the compilation phase, url encode the /. The server should already be able to handle that without any adapter changes. This may be the best option as there is quite a bit less magic then the proposed solution. The main downside is kind of ugly urls, but I can live with that, especially since this is somewhat of an edge case.

  3. Yell loudly enough that AWS cares to fix this.

Importance

would make my life easier

Additional Information

I'll update this if anything comes back from the lambda team, but I figure it would be unhealthy to hold my breath.

@PatrickG
Copy link
Member

PatrickG commented Sep 30, 2022

4. Just thought of this one. There could be a SvelteKit config option to, in the compilation phase, url encode the /. The server should already be able to handle that without any adapter changes. This may be the best option as there is quite a bit less magic then the proposed solution. The main downside is kind of ugly urls, but I can live with that, especially since this is somewhat of an edge case.

That's the first thing that came to mind, but it would not work with something like:

<script>
  import { action } from 'wherever';
</script>

<form {action}>...</form>

@CalebBassham
Copy link
Author

I guess maybe a doing the transformation at render-time instead of compile-time would be better then. Or a mix. Optimistically compile-time, but fallback to render-time if there is a case like that.

@repsac-by
Copy link
Contributor

rfc2396 is obsolete, see rfc3986 3.4

@CalebBassham
Copy link
Author

Good find. I will add this to my AWS ticket.

@Rich-Harris
Copy link
Member

I can't reproduce this. Here's an app deployed with sveltekit-adapter-aws — submitting the form with an ?/echo action works just fine:

https://d2tcem8te28uok.cloudfront.net/test

The same app works fine on Vercel in Lambda mode:

https://sveltejs-kit-7098.vercel.app/test

What am I missing? If this is somehow broken on Lambda it's a huge bug on their part; I'd be astonished if we were the first to encounter it!

@CalebBassham
Copy link
Author

The difference is that sveltekit-adapter-aws is connecting the lambda function to an API Gateway and then to Cloudfront. In this case, I am using a Lambda function URL and connecting that to Cloudfront (no API Gateway).

API Gateway used to be the only way to call a Lambda function with a url, but now you can configure any lambda function to have an url endpoint using the function urls. That does not make API Gateway irrelevant because it is still better if you want to connect multiple functions to handle different endpoints on the same API. However, SK only uses one handler, so a function url is much simpler to setup and less AWS resources to have to manage for what should be the same result. But, as we see here there are some differences.

Still working through this with people from AWS too, but it is slow going.

@Rich-Harris
Copy link
Member

Ah, gotcha. I'm going to close this issue in that case, as there's really nothing SvelteKit can do about this other than break the design, and I don't think it makes sense to do that just because of a bug in Lambda when it's relatively straightforward to work around. FWIW the 'one handler' thing is just the way sveltekit-adapter-aws works, but it's possible to generate multiple functions for a single app (it's entirely up to the adapter — adapter-vercel and adapter-netlify both have a split: true option, for example; the same could be added to sveltekit-adapter-aws, in which case API Gateway would presumably be more useful). Good luck dealing with AWS!

@Rich-Harris Rich-Harris closed this as not planned Won't fix, can't repro, duplicate, stale Nov 1, 2022
@CalebBassham
Copy link
Author

Sure. If anyone stumbles upon this in the future, I made a repro that modifies sveltekit-adapter-aws to use a function url here https://github.com/CalebBassham/lambda-function-url-repro. Currently it is also deployed to https://d1wbljbjs6dwlg.cloudfront.net/sverdle and you can see it failing when it tries to submit a sverdle guess. Probably won't be deployed there once this is fixed though, but I guess it doesn't really matter at that point. Maybe sooner. Deployment instructions included in repro repo.

@CalebBassham
Copy link
Author

FWIW the 'one handler' thing is just the way sveltekit-adapter-aws works, but it's possible to generate multiple functions for a single app (it's entirely up to the adapter — adapter-vercel and adapter-netlify both have a split: true option, for example; the same could be added to sveltekit-adapter-aws, in which case API Gateway would presumably be more useful).

Is there an advantage to splitting them out? Smaller bundle size I would assume? How trivial is it for an adapter to add that?

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.

4 participants