In the network panel copy posted data or response data in binary format doesn't work correctly

ASSIGNED
Assigned to

Status

P3
normal
ASSIGNED
2 years ago
3 months ago

People

(Reporter: aquilax, Assigned: locke12456)

Tracking

({good-first-bug})

50 Branch
good-first-bug

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(6 attachments)

(Reporter)

Description

2 years ago
Created attachment 8829268 [details]
RequestPayload.png

User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:50.0) Gecko/20100101 Firefox/50.0
Build ID: 20161208153507

Steps to reproduce:

I'm testing an application sending and receiving binary data from the server. I need to test if the sent binary data is correct and I wanted to copy the sent binary data from the network panel. I tried to use the context menu on the request "Copy POST Data" and to select the text in the "Request payload" and paste it in a text processor like notepad++.


Actual results:

The binary data is cut, the paste function returns only the text until the char code \u0
I don't know if it is a problem of firefox or of the copy paste function.


Expected results:

Be able to export the whole binary data in the request payload (posted data) as well as in the response.

Updated

2 years ago
Component: Untriaged → Developer Tools: Netmonitor
Keywords: good-first-bug
Priority: -- → P3
(Assignee)

Comment 1

2 years ago
Hi Honza
I'd like work on this issue. Could you please assign to me?
Thank you.:)
Flags: needinfo?(odvarko)
Assignee: nobody → locke12456
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Flags: needinfo?(odvarko)
(Assignee)

Comment 2

2 years ago
Created attachment 8859038 [details]
Copy POST data.png

Hi Honza
Looks this issue is not occur on netmonitor.
I tried to duplicate it however it hasn't happen. but I found another issue,I think this issue is occurred in copyString method.

Please find the file attached.
This method copied the incomplete data.
In [3], here is the output of the postData.
So far the data is correct, but the copyString is not. see [4].
Flags: needinfo?(odvarko)
(In reply to Locke Chen from comment #2)
> Created attachment 8859038 [details]
> Copy POST data.png
> 
> Hi Honza
> Looks this issue is not occur on netmonitor.
> I tried to duplicate it however it hasn't happen. but I found another
> issue,I think this issue is occurred in copyString method.
> Please find the file attached.
> This method copied the incomplete data.
> In [3], here is the output of the postData.
> So far the data is correct, but the copyString is not. see [4].
Yes, sounds like it could be the problem. Perhaps we need to encode the data first (making sure it is *not* binary) before passing into copyString() ? It should happen only for binary responses I guess.
Is your test page online so, I can also try it?
Honza
Flags: needinfo?(odvarko)
(Assignee)

Comment 5

2 years ago
(In reply to Jan Honza Odvarko [:Honza] from comment #4)
> (In reply to Locke Chen from comment #2)
> > Created attachment 8859038 [details]
> > Copy POST data.png
> > 
> > Hi Honza
> > Looks this issue is not occur on netmonitor.
> > I tried to duplicate it however it hasn't happen. but I found another
> > issue,I think this issue is occurred in copyString method.
> > Please find the file attached.
> > This method copied the incomplete data.
> > In [3], here is the output of the postData.
> > So far the data is correct, but the copyString is not. see [4].
> Yes, sounds like it could be the problem. Perhaps we need to encode the data
> first (making sure it is *not* binary) before passing into copyString() ? It
> should happen only for binary responses I guess.
> Is your test page online so, I can also try it?
> Honza

Hi Honza
Agree, I'll try to encode the data first.
You can upload some file to MEGA.nz and open the netmonitor.
https://mega.nz/
Thank you. :)
Flags: needinfo?(odvarko)
(Assignee)

Comment 6

2 years ago
Created attachment 8859428 [details]
test result.txt

I tested encode the binary data to JSON format. 
Looks data is correct. see [1](in file attached - test result.txt).
And I also copied HAR's post data. see [2].
(In reply to Locke Chen from comment #6)
> Created attachment 8859428 [details]
> test result.txt
> 
> I tested encode the binary data to JSON format. 
> Looks data is correct. see [1](in file attached - test result.txt).
> And I also copied HAR's post data. see [2].
Yep, looks promising! Any patch(es) available for review?

Honza
Flags: needinfo?(odvarko)
(Assignee)

Comment 8

2 years ago
I'm thinking about how to verify that postdata is a binary file.
Looks this issue happen only from binary data.
Copied from "copy POST data" method also having quote character if I just using JSON.stringify method.
it will look likes "íí\u0019œ•\u0014ÆUߨ”òêGm\u001d÷Ì̓XIlu0áÅõ\n\u0016-:!k½dûú¸H\nþSß\u0006üñӟ$G\u0006 ..".

Could you please give some suggestions to me?
Thanks!
Flags: needinfo?(odvarko)
(In reply to Locke Chen from comment #8)
> Could you please give some suggestions to me?
Good question. We might have to iterate over the data and look for non-text characters. In any case, implementation of any helper function should happen in bug 752288. Also, you can ask Mike Ratcliffe, I recall he had some ideas while back.

Honza
Flags: needinfo?(odvarko)
(Assignee)

Comment 10

2 years ago
Thank you Honza.
Maybe I can divide it into two cases (or more).
Plain text or binary, but I have to try.
(Assignee)

Comment 11

2 years ago
We seem to only need to detect 0x00 symbols.
Binary has many 0x00 symbols, but plain text files are not.
Looks that copyString issue reason is 0x00 symbol, so there not copy.
Could I add method to verify binary or text?

I will be fix at here:
https://searchfox.org/mozilla-central/source/devtools/client/netmonitor/src/request-list-context-menu.js#249-254
Flags: needinfo?(odvarko)
(In reply to Locke Chen from comment #11)
> We seem to only need to detect 0x00 symbols.
Yes this sounds like a good simple check.

Can we do more than just 0x00?
Perhaps we could search for some other non-printable chars?

See couple of examples in our code base:

http://searchfox.org/mozilla-central/rev/ae8c2e2354db652950fe0ec16983360c21857f2a/devtools/client/debugger/new/integration-tests.js#1735

function nonprintable(code) { return between(code, 0,8) || code == 0xb || between(code, 0xe,0x1f) || code == 0x7f; }

http://searchfox.org/mozilla-central/rev/ae8c2e2354db652950fe0ec16983360c21857f2a/devtools/client/shared/components/reps/reps.js#409

function sanitizeString(text) {
  // Replace all non-printable characters, except of
  // (horizontal) tab (HT: \x09) and newline (LF: \x0A, CR: \x0D),
  // with unicode replacement character (u+fffd).
  // eslint-disable-next-line no-control-regex
  let re = new RegExp("[\x00-\x08\x0B\x0C\x0E-\x1F\x7F-\x9F]", "g");
  return text.replace(re, "\ufffd");
}

Honza
Flags: needinfo?(odvarko)
(Assignee)

Comment 13

2 years ago
(In reply to Jan Honza Odvarko [:Honza] from comment #12)
> (In reply to Locke Chen from comment #11)
> > We seem to only need to detect 0x00 symbols.
> Yes this sounds like a good simple check.
> 
> Can we do more than just 0x00?
> Perhaps we could search for some other non-printable chars?
> 
> See couple of examples in our code base:
> 
> http://searchfox.org/mozilla-central/rev/
> ae8c2e2354db652950fe0ec16983360c21857f2a/devtools/client/debugger/new/
> integration-tests.js#1735
> 
> function nonprintable(code) { return between(code, 0,8) || code == 0xb ||
> between(code, 0xe,0x1f) || code == 0x7f; }
> 
> http://searchfox.org/mozilla-central/rev/
> ae8c2e2354db652950fe0ec16983360c21857f2a/devtools/client/shared/components/
> reps/reps.js#409
> 
> function sanitizeString(text) {
>   // Replace all non-printable characters, except of
>   // (horizontal) tab (HT: \x09) and newline (LF: \x0A, CR: \x0D),
>   // with unicode replacement character (u+fffd).
>   // eslint-disable-next-line no-control-regex
>   let re = new RegExp("[\x00-\x08\x0B\x0C\x0E-\x1F\x7F-\x9F]", "g");
>   return text.replace(re, "\ufffd");
> }
> 
> Honza

Oh, that's great. thank you for your hint.
I'll try it first.
(Assignee)

Comment 14

2 years ago
Looks I can't reuse the nonprintable method.
Should I add these method to utils? (like the nonprintable,between .. )
I also need to use Blob and FileReader(to get ArrayBuffer, maybe also async and await.).
It will be a long method if I just put these code on here.
Flags: needinfo?(odvarko)
(In reply to Locke Chen from comment #14)
> Looks I can't reuse the nonprintable method.
> Should I add these method to utils? (like the nonprintable,between .. )
> I also need to use Blob and FileReader(to get ArrayBuffer, maybe also async
> and await.).
> It will be a long method if I just put these code on here.
I think that it's OK if you implement your own method.
(you might want to put it into the utils/request-utils.js file)

Honza
Flags: needinfo?(odvarko)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 18

2 years ago
Changes:
 * fixed copyString not work when postData is binary string. 
 * added method to utils/request-utils.js to verification binary when post data can't parsing to JSON.
 * https://searchfox.org/mozilla-central/source/devtools/client/netmonitor/src/request-list-context-menu.js#249-254

Comment 19

2 years ago
mozreview-review
Comment on attachment 8865420 [details]
Bug 1332949 - In the network panel copy posted data or response data in binary format doesn't work correctly.

https://reviewboard.mozilla.org/r/137074/#review140082

Thanks for the patch!

Please see my inline comments.

Honza

::: devtools/client/netmonitor/src/utils/request-utils.js:312
(Diff revision 2)
> +
> +function nonprintable(code) {
> +  return between(code, 0, 8) || code == 0xb || between(code, 0xe, 0x1f) || code == 0x7f;
> +}
> +
> +async function verificationBinaries(string) {

Could we rename the method to `isBinary` ?

::: devtools/client/netmonitor/src/utils/request-utils.js:314
(Diff revision 2)
> +  return between(code, 0, 8) || code == 0xb || between(code, 0xe, 0x1f) || code == 0x7f;
> +}
> +
> +async function verificationBinaries(string) {
> +  let blob = new Blob([string], {type: "application/octet-binary"});
> +  return new Promise((reslove, reject) => {

Typo `reslove` => `resolve`

::: devtools/client/netmonitor/src/utils/request-utils.js:328
(Diff revision 2)
> +      let buff = reader.result;
> +      let view = new Uint8Array(buff);
> +
> +      for (let code in view) {
> +        if (nonprintable(code)) {
> +          reslove(true);

Typo `reslove` => `resolve`

::: devtools/client/netmonitor/src/utils/request-utils.js:333
(Diff revision 2)
> +          reslove(true);
> +          return true;
> +        }
> +      }
> +
> +      reslove(false);

Typo `reslove` => `resolve`
Attachment #8865420 - Flags: review?(odvarko)
Comment hidden (mozreview-request)

Comment 21

2 years ago
mozreview-review
Comment on attachment 8865420 [details]
Bug 1332949 - In the network panel copy posted data or response data in binary format doesn't work correctly.

https://reviewboard.mozilla.org/r/137074/#review140440

LGTM, thanks!

R+

Honza
Attachment #8865420 - Flags: review?(odvarko) → review+
(Assignee)

Comment 22

2 years ago
Looks like some test cases fail. I will try to fix it.
(Assignee)

Comment 23

2 years ago
Hi Honza

How can I run this test on my computer? I got test failed, but I have no idea to duplicate this. 
It not happen on my machine. I have tried 'mach mochitest devtools/client/netmonitor', but all test passed. :(
https://treeherder.mozilla.org/logviewer.html#?job_id=97589824&repo=try&lineNumber=4921
Flags: needinfo?(odvarko)
When I run the following *without* the patch it works

`mach test devtools/client/netmonitor/test/browser_net_copy_params.js`

When I run it *with* the patch I get the same error as try on Linux 64 (I am on Win10)

172 INFO TEST-UNEXPECTED-FAIL | devtools/client/netmonitor/test/browser_net_copy_params.js | Timed out while polling clipboard for pasted data -
Stack trace:
chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:wait:1003
chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForClipboard/wait/<:1022
173 INFO TEST-UNEXPECTED-FAIL | devtools/client/netmonitor/test/browser_net_copy_params.js | Uncaught exception -
Buffered messages finished
SUITE-END | took 17s

---

Can you try the same?

Perhaps the data copied into the clipboard are too big? 
What if you make is smaller?

Honza
Flags: needinfo?(odvarko)
(Assignee)

Comment 25

2 years ago
Thank you, now I can see that.
But have we tested copy row data in these test case?
If I removed the verify statement it will pass.
I have no idea why tests enter this scope:
https://searchfox.org/mozilla-central/source/devtools/client/netmonitor/src/request-list-context-menu.js#253-257

Thank you.
Flags: needinfo?(odvarko)
(Assignee)

Comment 26

2 years ago
Oh, sorry I found bug from my code.
I have changed from
for (let code in view) {
   if (nonprintable(code)) 
to
for (let count = 0 ;count < view.length; count++ )
   if (nonprintable(view[count]))

It's a stupid bug. XD

Should I also add one test for binaries?
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(In reply to Locke Chen from comment #26)
> Should I also add one test for binaries?
Yes please!

Honza
Flags: needinfo?(odvarko)
(Assignee)

Comment 30

2 years ago
Hi Honsa
I have been tried to add one test case for this feature, but looks like this feature can't be pass the 'Timeout'.
http://searchfox.org/mozilla-central/source/testing/mochitest/tests/SimpleTest/SimpleTest.js#1022

Currently I have used "Promise" to wait the parsing binary data to Array. (Blob > FileReader > ArrayBuffer > verification )
In this case should be more than 100 milliseconds.
So I can't just use the copyCilpboard method to verify binary data.
Could I checked-in the patch first? :(
Flags: needinfo?(odvarko)
Does it help if you use `requestLongerTimeout`?

An enxample:
http://searchfox.org/mozilla-central/source/devtools/client/aboutdebugging/test/browser_addons_debug_bootstrapped.js#6

Honza
Flags: needinfo?(odvarko)
(Assignee)

Comment 32

a year ago
Well, thanks. But still the same.
I always got this message:

185 INFO TEST-UNEXPECTED-FAIL | devtools/client/netmonitor/test/browser_net_copy_params.js | Timed out while polling clipboard for pasted data - 
Stack trace:
chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:wait:1003
chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForClipboard/wait/<:1022
GECKO(6203) | Got this value: "R$Y”\f˜$R÷\u0018\u0002\u0018:¼ú̶±,I웅Ôï ¤üCz"
186 INFO TEST-UNEXPECTED-FAIL | devtools/client/netmonitor/test/browser_net_copy_params.js | Uncaught exception - 

The string from the clipboard is correct, I still don't know what happened here.
Flags: needinfo?(odvarko)
Did you try to increase the timeout for SimpleTest.waitForClipboard?

You can pass custom timeout into SimpleTest.waitForClipboard and increase also maxPolls number, see here:
http://searchfox.org/mozilla-central/source/testing/mochitest/tests/SimpleTest/SimpleTest.js#988

You might want to also modify 
http://searchfox.org/mozilla-central/source/devtools/client/framework/test/shared-head.js#519

Honza
Flags: needinfo?(odvarko)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 36

a year ago
Thanks, but I still got test fail. :(
Cloud you please help to me review the test case?(Maybe I used the wrong method.)
Flags: needinfo?(odvarko)
@Michal: can you please take a quick look at this change?

Honza
Flags: needinfo?(odvarko) → needinfo?(michal.novotny)
@Michal: sorry, wrong bug

Setting NI back to me.

Honza
Flags: needinfo?(michal.novotny) → needinfo?(odvarko)
(Reporter)

Comment 39

a year ago
Created attachment 8886193 [details]
CopyPasteBinaryData.png

Upper part network panel in FF
Lower part paste in notepad++
(Reporter)

Comment 40

a year ago
Hello, I had to check some other binary requests and I though to give a try to FF if it was working and actually it copies the whole "text" (s. picture), but the pasted content is somehow incorrect with the encoding. In the picture is also visible the content-length, which is 82 chars, which is correct in the network panel of FF, but when pasted in notepad++ they are partially different and many more. I suppose it is a encoding problem, notepad++ support some like utf7 and utf8 but with both wasn't working. The only way to obtain the same chars in a word processor is to save all the requests in json format (HAR) and when you open it in notepad++ the chars are correct ... but the problem is the json encoding of the special chars like \U0001 etc. So also the export in json format is not directly usable, I had to load the file in programming language, I used powershell, which decode the \U0001 encoding to chars and then export them again in a new file. At the end I was able to "copy and paste" the binary data from the network panel, although I had to export all the request in json, import the exported json file in powershell, select the nodes with the requests and export them in plain text file.
It would be great if the encoding mismatching could be solved, it would make the life of few developers easier ;)

Updated

4 months ago
Product: Firefox → DevTools

Comment 41

3 months ago
mozreview-review
Comment on attachment 8870686 [details]
Bug 1332949 - test case for copy binary data.

https://reviewboard.mozilla.org/r/142152/#review262480

I am unassigning myself since this needs general rebase.
Attachment #8870686 - Flags: review?(odvarko)
@Locke: still around?

Honza
Flags: needinfo?(odvarko) → needinfo?(locke12456)
(Assignee)

Comment 43

3 months ago
Hi Honza,
yap, long time no see. :)
Flags: needinfo?(locke12456)
You need to log in before you can comment on or make changes to this bug.