-
Notifications
You must be signed in to change notification settings - Fork 10
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
validate_asset_path should not "leak" exception in case of user's regex misspecification #1913
Comments
This is the tail of the invocation log on my end. The error code is
|
thank you! it is those |
This is everything that got 503 → https://ppb.chymera.eu/aa98fa |
@jwodder -- from above log could you figure out what specific asset/path triggrered those
if not -- could we (in dandi-cli) then log more there for POST , e.g. what path etc? |
Perhaps if I had the full log rather than a filtered-down version (CC @TheChymera).
The logging of failed requests is endpoint-agnostic, so the only way to log the path would be to log the entire request body for all failed POSTs, which would quite often be too much information. (It's also conceivable that, when failing to create an asset in an embargoed Dandiset, this approach could end up logging NWB fields that should arguably be kept secret.) |
@jwodder the whole log is over 200MB, uploaded it here → https://ppb.chymera.eu/471899.log |
@yarikoptic @TheChymera The assets that produced the 503 were those whose paths contained a |
I can reproduce the 503 error when just trying to create an asset with the same path as one of the failing ones, but if I try something simpler like |
Look at SEntry report from today (says id ee7345f8563743bfb9f288c1fbea145e) where @TheChymera (congrats! Achievement unlocked ;) ) managed to make SEntry to react and log traceback
so feels like that invocation should be guarded and some correct HTTP error code be returned instead of server "blowing up" with some 5xx error (I guess that is what happened, @TheChymera might be able to confirm)
The text was updated successfully, but these errors were encountered: