Replace 404 response with 403 and 500 when appropriate#18
Replace 404 response with 403 and 500 when appropriate#18hueniverse merged 1 commit intohapijs:masterfrom
Conversation
lib/file.js
Outdated
There was a problem hiding this comment.
No need for else.
Is the | a typo for ||?
There was a problem hiding this comment.
I used a single | to retain code coverage. The new tests will trigger EACCES on linux but on windows EPERM is triggered instead. Afaik both values indicate a permission issue.
There was a problem hiding this comment.
ugh. you should find another way.
There was a problem hiding this comment.
Ok, I fixed it with additional mocking. I suspect a more proper solution would be to write a new module that converts syserr to Boom errors and use that instead. Any thoughts on such a module @arb ?
de55d05 to
811d51f
Compare
There was a problem hiding this comment.
Style nit: Capitalize failed, also line 205 missing space after function
Replace 404 response with 403 and 500 when appropriate
|
This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions. |
This fixes a somewhat serious issue where clients would get a
404responses whenopenfails because ofEMFILE, etc. This should not happen for this kind of ephemeral error.The patch will now only return
404when the filesystem reportsENOENT. Otherwise, it will detect permission errors and return403responses to indicate this, and finally it can fail with an internal error, exposing the actual error.