Add support for HTTP Archive (HAR) by gusdantas · Pull Request #696 · ChuckerTeam/chucker

gusdantas · 2021-09-24T16:34:53Z

📷 Screenshots

📄 Context

This updates @JayNewstrom 's solution which allows Chucker to share/export files with desktop http viewers such as Charles Proxy, Chrome Network Debugger, etc.

Closes #401

📝 Changes

Most of the code is new, but some changes were made on toolbars export buttons to allow har export.

🚫 Breaking

No breaking changes.

🛠️ How to test

@JayNewstrom 's solution has some automated tests. You can also test via the UI (see screenshots). After you have the file ready to import into another application, it should be able to be imported.

⏱️ Next steps

As told by @JayNewstrom:

This handles the simple case, and allows you to view request/response in Charles. The generated file is likely not 100% accurate/to spec yet. See comments in #401 for some reasons why (ie. missing data).

I'd like to ensure this is the right direction, and get bugs/examples that fail to import and fix them as we find them.

I'd also like to add support for binary request/response bodies via base 64 encoding.

MiSikora · 2021-10-07T18:38:25Z

Sorry for the radio silence. I'm currently preoccupied with life stuff. I should have more time soonish to take a look at this PR.

cortinico

That's great 🚀 Thanks for taking the time to do this. I've left several comments. Sorry for taking longer to review.

I'd also like other maintainers to chime in if possible.

Also as a side note for the future: the diff size was quite big. Having something smaller (like 2/3 PRs instead) would have speeded up the whole process.

MiSikora

I remember there were some issues with usability, so these should be checked before merging.

#417 (comment)

vbuberen

I think I would leave the rest of review to @cortinico and @MiSikora as they already jumped in here and left quite a lot of feedback.

From my side the only request is to also update the CHANGELOG file to have this new feature mentioned.

Thanks a lot @gusdantas for picking up this feature request.

…nsion, and fix mimeType

…update changelog

MiSikora · 2021-10-25T17:27:09Z

One more thing that just caught my eye. Should we do anything special about headers? Chucker can redact them on demand. I think it's ok to show them as redacted in .har file but what about the size? Won't this be misleading to the users?

gusdantas · 2021-10-26T00:17:24Z

One more thing that just caught my eye. Should we do anything special about headers? Chucker can redact them on demand. I think it's ok to show them as redacted in .har file but what about the size? Won't this be misleading to the users?

Sorry @MiSikora, I didn't get your concerns about the headers... Could you please give an example of possible issue if it's not too much trouble?

MiSikora · 2021-10-26T06:47:16Z

Could you please give an example of possible issue if it's not too much trouble?

Authorization: Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c

The correct byte count for such a header is 179. However, when we redact it using ChukcerInterceptor it will be 19 because the redacted value is changed to **.

I don't see it as a big problem because that is functionality that users willingly enable. Just something I'd like to get feedback on.

gusdantas · 2021-10-26T20:33:11Z

...
The correct byte count for such a header is 179. However, when we redact it using ChukcerInterceptor it will be 19 because the redacted value is changed to **.
...

Oh, now it is clear, thanks @MiSikora !
Particularly, I see no problem in show redacted field with the correct byte count. But if anyone has concerns about it, I can change it.

vbuberen · 2021-10-27T12:18:49Z

Particularly, I see no problem in show redacted field with the correct byte count. But if anyone has concerns about it, I can change it.

Under correct you mean the byte count which will be counted with redacted value? If so, I am also fine with it, as it supposed that user intentionally used feature to hide some headers. The only question arises is the case when user passes HAR file to somebody else from the team and forgets to notify about changes applied, but I believe it is really rare case and we shouldn't worry that much about it.

P.S. I would like to remind you about my request in one of previous comments:

From my side the only request is to also update the CHANGELOG file to have this new feature mentioned.

gusdantas · 2021-10-27T14:18:37Z

Under correct you mean the byte count which will be counted with redacted value? If so, I am also fine with it, as it supposed that user intentionally used feature to hide some headers. The only question arises is the case when user passes HAR file to somebody else from the team and forgets to notify about changes applied, but I believe it is really rare case and we shouldn't worry that much about it.

@vbuberen actually I was thinking to pass the real byte count received, not the redacted one (in the example, it would be a "heuristic" to know that the field is fine, like, I cannot know what is in that field but I know it may be ok because it has the same size of an expected one). Nevertheless, if it is preferable to show the byte count redacted I should do this change.

P.S. I would like to remind you about my request in one of previous comments:

From my side the only request is to also update the CHANGELOG file to have this new feature mentioned.

It is already done. =D

MiSikora · 2021-10-27T14:51:10Z

Nevertheless, if it is preferable to show the byte count redacted I should do this change.

The current implementation will show a redacted byte count. IMO this is not preferable.

I will do some integration tests this weekend, and if everything is ok, it will be good to go from my side.

cortinico · 2021-10-27T23:21:16Z

The current implementation will show a redacted byte count. IMO this is preferable.

Why is it prerrable to show the redacted byte count? From my point of view, redacting should just affect the "viewing" of the headers, but all the other properties should not be affected. I think not tweaking the byte count will be preferable and will introduce less confusion, but I'm interested to hear other opinion.

I will do some integration tests this weekend, and if everything is ok, it will be good to go from my side.

Awesome 🙏 Looking forward to it.

MiSikora · 2021-10-28T04:12:10Z

Why is it prerrable to show the redacted byte count? From my point of view, redacting should just affect the "viewing" of the headers, but all the other properties should not be affected. I think not tweaking the byte count will be preferable and will introduce less confusion, but I'm interested to hear other opinion.

A typo. I meant not preferable. That's why I even asked about it in the first place. 🙃

Nothing blocking though.

gusdantas · 2021-10-28T20:59:00Z

Dears,
Now the header size is computed before redact.

gusdantas · 2021-10-29T19:03:04Z

@MiSikora I added a test on ChuckerInterceptorTest but I couldn't find a way to get the request when the ClientFactory is NETWORK. Is there a correct way to do it?

MiSikora · 2021-10-29T20:21:35Z

You should be able to take it from the server. Something like server.takeRequest().headers.byteCount(). Or response.networkResponse!!.request.headers.byteCount().

gusdantas · 2021-10-29T22:02:14Z

You should be able to take it from the server. Something like server.takeRequest().headers.byteCount(). Or response.networkResponse!!.request.headers.byteCount().

@MiSikora thank you, now the test passes on both ClientFactory!

MiSikora · 2021-10-29T23:16:28Z

Left 1 NIT. I'll do tests tomorrow to see how it behaves, and hopefully, we're good to go. 👍

MiSikora

Ok, I did some tests using Chucker both as an application interceptor and a network interceptor. For reading HAR files, I used this application - https://toolbox.googleapps.com/apps/har_analyzer.

Thank you for this contribution! I didn't find any issues, and everything worked as expected. After changing one last NIT and after @cortinico gives their approval, I think we can merge it.

cortinico · 2021-10-31T11:41:31Z

Great work 🚀 Thanks for tackling this

gusdantas · 2021-10-31T13:08:33Z

@cortinico @MiSikora @vbuberen I do appreciate all the compliments, but the main effort was made by @JayNewstrom !
Thank you!

Update har export implementation

7806c48

gusdantas mentioned this pull request Sep 24, 2021

Add support for exporting requests to HTTP Archive format (.har) #401

Closed

2 tasks

Fix lint pre merge error

620c552

cortinico requested changes Oct 16, 2021

View reviewed changes

cortinico added the enhancement New feature or improvement to the library label Oct 16, 2021

gustavo-hidalgo-gok added 4 commits October 17, 2021 15:24

Fix har data constructors

817e9aa

Fix test names

109ddb4

Retrieve byte count from headers

f083113

Delete unused file and fix date format for har file

977d77d

cortinico reviewed Oct 18, 2021

View reviewed changes

Comment thread library/src/test/java/com/chuckerteam/chucker/util/TestTransactionFactory.kt

Add comment to Timings

535d36a

MiSikora suggested changes Oct 23, 2021

View reviewed changes

gustavo-hidalgo-gok added 2 commits October 23, 2021 19:13

Rename methods

931bc91

modify QueryString and TransactionDetailsHarSharable

0665fa7

vbuberen reviewed Oct 24, 2021

View reviewed changes

gustavo-hidalgo-gok added 3 commits October 24, 2021 19:35

Create requestBodySize and responseBpdySize, modify harFormatted exte…

a6bbe62

…nsion, and fix mimeType

Remove main thread blocking, change json converter instantiation and …

714c115

…update changelog

Merge branch 'develop' of github.com:gusdantas/chucker into feature/har

0e90f34

MiSikora reviewed Oct 25, 2021

View reviewed changes

Use requestPayloadSize and responsePayloadSize

c2cd4c1

Change exporting file logic

06e8e5b

Compute header size before redact

4a65043

MiSikora reviewed Oct 29, 2021

View reviewed changes

Comment thread library/src/main/java/com/chuckerteam/chucker/internal/data/entity/HttpTransaction.kt Outdated

Comment thread library/src/main/java/com/chuckerteam/chucker/internal/data/entity/HttpTransaction.kt Outdated

Modify headers size filling

0a23100

Fix ChuckerInterceptorTest

86cdbb0

MiSikora reviewed Oct 29, 2021

View reviewed changes

Comment thread library/src/test/java/com/chuckerteam/chucker/api/ChuckerInterceptorTest.kt Outdated

Modify header size test

7ed8c95

MiSikora reviewed Oct 30, 2021

View reviewed changes

Comment thread library/src/test/java/com/chuckerteam/chucker/api/ChuckerInterceptorTest.kt Outdated

MiSikora approved these changes Oct 30, 2021

View reviewed changes

gustavo-hidalgo-gok added 2 commits October 30, 2021 10:06

Modify headers size test

9aff57d

Change headers size test logic

bd091c6

cortinico approved these changes Oct 31, 2021

View reviewed changes

cortinico merged commit 218a997 into ChuckerTeam:develop Oct 31, 2021

gusdantas deleted the feature/har branch October 31, 2021 12:50

vbuberen mentioned this pull request Nov 2, 2021

Request size is shown as -1 #714

Closed

cortinico mentioned this pull request Dec 30, 2021

Add Support for Custom Shareable Options #744

Open

2 tasks

BluestormDNA mentioned this pull request Aug 27, 2022

Add export function to API (.har file) #871

Closed

2 tasks

Uh oh!

Conversation

gusdantas commented Sep 24, 2021 • edited by cortinico Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📷 Screenshots

📄 Context

📝 Changes

🚫 Breaking

🛠️ How to test

⏱️ Next steps

Uh oh!

MiSikora commented Oct 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cortinico left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MiSikora left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vbuberen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MiSikora commented Oct 25, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gusdantas commented Oct 26, 2021

Uh oh!

MiSikora commented Oct 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gusdantas commented Oct 26, 2021

Uh oh!

vbuberen commented Oct 27, 2021

Uh oh!

gusdantas commented Oct 27, 2021

Uh oh!

MiSikora commented Oct 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cortinico commented Oct 27, 2021

Uh oh!

MiSikora commented Oct 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gusdantas commented Oct 28, 2021

Uh oh!

Uh oh!

Uh oh!

gusdantas commented Oct 29, 2021

Uh oh!

MiSikora commented Oct 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

gusdantas commented Sep 24, 2021 •

edited by cortinico

Loading

MiSikora commented Oct 7, 2021 •

edited

Loading

MiSikora commented Oct 25, 2021 •

edited

Loading

MiSikora commented Oct 26, 2021 •

edited

Loading

MiSikora commented Oct 27, 2021 •

edited

Loading

MiSikora commented Oct 28, 2021 •

edited

Loading

MiSikora commented Oct 29, 2021 •

edited

Loading