Skip to content

Conversation

@chris48s
Copy link
Member

@chris48s chris48s commented Apr 4, 2025

We have a number of endpoints that allow the user to supply a URL to call.

We do impose a hard limit on the size of payload that a user can force us to download and parse, but beyond the global request timeout there's no limit on how long a user can force us to hold a connection open for.

On these endpoints where the user can give us an arbitrary URL to call, I propose we put a shorter timeout on how long we are prepared to wait for the upstream request. 3.5 seconds is still quite a long time if we are getting a flood of requests like that, but I feel like in the situation where the user supplies the URL we should be more pessimistic than when we are calling a known reputable host.

@chris48s chris48s added the service-badge New or updated service badge label Apr 4, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Apr 4, 2025

Warnings
⚠️ This PR modified service code for dynamic but not its test code.
That's okay so long as it's refactoring existing code.
Messages
📖 ✨ Thanks for your contribution to Shields, @chris48s!

Generated by 🚫 dangerJS against 5a7a22d

@chris48s
Copy link
Member Author

TODO: update for #10985

Copy link
Member

@calebcartwright calebcartwright left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assume the todo in #10996 (comment) is sorted now given the dynamic-regex timeout has the timeout change too

can re- 👍 if there's more pending though

@chris48s
Copy link
Member Author

chris48s commented May 1, 2025

Yes. I already updated it. Thanks

@chris48s chris48s added this pull request to the merge queue May 1, 2025
Merged via the queue into badges:master with commit 732b06b May 1, 2025
23 checks passed
@chris48s chris48s deleted the endpoint-limit branch May 1, 2025 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

service-badge New or updated service badge

Development

Successfully merging this pull request may close these issues.

2 participants