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

Prevent users from iterating a request's body after having responded to that request #382

Open
Joannis opened this issue Feb 15, 2024 · 7 comments

Comments

@Joannis
Copy link
Member

Joannis commented Feb 15, 2024

Currently, we implement an HBRequest's Body with the NIOAsyncChannel's inbound. This is efficient, but might lead to users escaping the request.body for later consumption. This will lead to bugs.

@adam-fowler
Copy link
Member

Are you talking about iterating the request body in an unstructured task?

@Joannis Joannis closed this as completed Feb 20, 2024
@Joannis Joannis reopened this Feb 20, 2024
@Joannis
Copy link
Member Author

Joannis commented Feb 20, 2024

Exactly, but explicitly after the response was already returned

@adam-fowler
Copy link
Member

You could set the request body to have been iterated as soon as the response body has finished writing. So any subsequent attempts to iterate via an unstructured task will fail and provide a better indication as to why it failed.

@JonPulfer
Copy link
Contributor

I'm going to take a look at this for you. I'm at an intermediate stage with Swift, so will enjoy helping out while improving my skills 😃

@adam-fowler
Copy link
Member

adam-fowler commented Jul 6, 2024

I'm going to take a look at this for you. I'm at an intermediate stage with Swift, so will enjoy helping out while improving my skills 😃

Hi Jon, what I mention above has been implemented but it doesn't really resolve the issue completely as the compiler stills allows you to iterate the request body in an unstructured Task. At this point in time there isn't really anything in Swift to resolve this issue but Swift 6 has a number of features that might help (~Escapable, better control of concurrency isolation) but they aren't available just now.

@JonPulfer
Copy link
Contributor

Ah ok, thanks for letting me know :)

I’ll keep my eye open for something else I can take a look at.

@adam-fowler
Copy link
Member

Ah ok, thanks for letting me know :)

I’ll keep my eye open for something else I can take a look at.

I can't think of anything obvious at the moment. But contributions are always welcome

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

No branches or pull requests

3 participants