Fix NPE and add isDirectory check in slurp #669
Conversation
Cartmanishere
force-pushed
the
slurp-fix
branch
from
a155047 to
74353bb
Compare
| @@ -26,3 +26,10 @@ | |||
| (t/is (= ["application/octet-stream" {}] (:content-type resp))) | |||
| (t/is (str/starts-with? (:body resp) "#binary[location=")) | |||
| (t/is (str/ends-with? (:body resp) ",size=681]")))) | |||
|
|
|||
| (t/deftest test-directory | |||
There was a problem hiding this comment.
Don't we also need for whatever was causing the NPE?
There was a problem hiding this comment.
The problem was some URLs return nil when doing .getContentType. I couldn't really figure out why the URLs did not return a content type.
I thought it was because of redirection but not all redirected URLs return nil. Tried various HTTP return codes. No idea why this happens.
src/cider/nrepl/middleware/slurp.clj
Outdated
| @@ -43,7 +43,8 @@ | |||
| `[type {:as attrs}]`. This method normalizes RFC | |||
| compliant content-types to this form." | |||
| [^String content-type] | |||
| (if-let [match (re-find content-type-pattern content-type)] | |||
| (if-let [match (and content-type | |||
There was a problem hiding this comment.
Shouldn't this nil check be handled in the caller of normalize-content-type? I think it's weird to expect to receive a nil argument.
There was a problem hiding this comment.
Yeah. Makes sense.
|
Thanks for tackling this! I've left some small remarks. Don't forget to add a changelog entry as well. |
|
Thanks for tackling this! 🙇 |
Fix some issues in
slurpmiddleware as seen in #534