ngrunwald / ring-middleware-format

Ring middleware for parsing parameters and emitting responses in JSON or other formats
163 stars 49 forks source link

Fix wrap-format-params for CoyoteInputStream. #13

Closed r0man closed 11 years ago

r0man commented 11 years ago

Hi Nils,

I'm running my Ring application inside Immutant/Tomcat. There the call wrap-format-params fails because the (> (.available body) 0) predicate is somehow false. The call to (.available body) on an org.apache.catalina.connector.CoyoteInputStream returns 0. I'm not sure if this is correct.

However, I looked at other middleware like ring-json. They don't do such a check on the input stream and just slurp it. This patch removes the (> (.available body) 0) check in order to get ring-middleware-format working on Immutant/Tomcat.

Would you like to merge this into master and make a release?

Thanks, Roman.

r0man commented 11 years ago

Hi Nils,

any thoughts on this?

Roman.

ngrunwald commented 11 years ago

Sorry, I forgot about this. Thanks for the patch, I'm travelling right now, but I'll merge it this week-end.

residentsummer commented 11 years ago

Can't use middleware-format in immutant due to this bug. Does proposed commit has some flaws preventing merge?

tobias commented 11 years ago

Bump - any word on merging this patch? We've had another Immutant user report the same issue.

cap10morgan commented 11 years ago

I would also like to see this merged, as we're running into this bug. If there is additional work / research to do, I'm happy to take a crack at it.

ngrunwald commented 11 years ago

Sorry about the looong delay, I had a lot on my plate professionaly these last few months... I have merged your patch, but as I decided to play it safe just in case and check the length of the resulting array. It avoids some unnessecary work in any case. Tell me if this fixes the problem. I'll cut a 3.1 release very soon.