search

jgilfelt / chuck

An in-app HTTP inspector for Android OkHttp clients
Apache License 2.0
4.68k stars 452 forks source link

Added support for a maximum content length property. The property rep… #12

Closed eschlenz closed 7 years ago

eschlenz commented 7 years ago

…resents a threshold for content that should be saved verbatim, or not.

@jgilfelt

WHAT: This PR introduces a "max content length" property to the ChuckInterceptor. A request or response body that exceeds the new property's value will not be recorded. That is, there will still be a record of the transaction, but the content itself is not kept.

WHY: Chuck was crashing on me when I would try to go to the Chuck UI. I determined that the saved response body for a few of my requests were simply too large for SQLite to handle. In such scenarios, I still want to see the transaction in the log. I don't need to see the content however. It made sense to apply this new max content length property to requests as well.

The max content length property is essentially disabled by default (set to Integer.MAX_VALUE). The user can optionally set a max length that works for them.

MISC: I tried to stick with the existing code conventions. Thanks for putting this tool together. It's very handy.

jgilfelt commented 7 years ago

Thanks! I've finally got my mock web server setup to send responses large enough to break things, so I will look at this as soon as possible.

I looks like it is Android's CursorWindow that breaks first, whose hard limit is 2048 KB.

eschlenz commented 7 years ago

@joaocsousa @jgilfelt Thanks for the review. I've made the switch from Integer to Long per joaocsuousa's recommendation. I've responded to the other callout (whether or not to keep the partial result).

eschlenz commented 7 years ago

Fixed the merge conflicts as well.

eschlenz commented 7 years ago

@jgilfelt Saw your change requests. I'll knock that out tomorrow and hit you back.

eschlenz commented 7 years ago

@jgilfelt @joaocsousa

I have addressed the feedback you guys gave me:

  1. I have shadowed the maxContentLength function in the no-op library.
  2. I have removed the "content too big" properties that I had previously added to the HttpTransaction model.
  3. The content is now retained, but truncated if it is too long.
  4. I have set the default maxContentLength to a more reasonable (and logical) value of 250000L.

With maxContentLength left at its default value:

screen shot 2017-02-14 at 9 14 55 am

screen shot 2017-02-14 at 9 14 58 am

With maxContentLength set to 5 (for testing):

screen shot 2017-02-14 at 9 15 31 am

screen shot 2017-02-14 at 9 15 36 am

jgilfelt commented 7 years ago

@eschlenz Nice work! I've just updated the method comment to reflect the new truncation behaviour. Thanks for putting this together so quickly.

eschlenz commented 7 years ago

@jgilfelt No problem! Happy to help.