Closed Bug 1344511 Opened 7 years ago Closed 7 years ago

POST field starting with "?" doesn't show properly

Categories

(DevTools :: Netmonitor, defect, P3)

Product:
DevTools
Component:
Netmonitor
Version:
54 Branch
Type:
defect

Tracking

(firefox55 verified)

Status:
VERIFIED FIXED
Milestone:
Firefox 55
Tracking Flags:
Tracking Status
firefox55 --- verified

People

(Reporter: edznux, Assigned: brennan.brisad)

Details

(Keywords: good-first-bug, testcase, Whiteboard: [netmonitor-reserve])

Attachments

(3 files, 1 obsolete file)

report_firefox_post.html
305 bytes, text/html
Details
IE11 vs Fx52.png
64.96 KB, image/png
Details
bug1344511.patch
4.16 KB, patch
Honza
: review+
Details | Diff | Splinter Review 
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:54.0) Gecko/20100101 Firefox/54.0
Build ID: 20170303030202

Steps to reproduce:

I made a POST request with a field named "?test".

Step to reproduce :
Send an AJAX request (with JQuery for example) with a field starting with "?"
The server should support post request.

Simple test case here (and in attachment)
...
<body>
	Error in the network display tab
</body>
<script src="https://code.jquery.com/jquery-3.1.1.min.js"></script>
<script>
$.ajax({
	method: "POST",
	url: "/",
	data: "?ok=12"
}).done(function( msg ) {
	console.log(msg)
});
</script>
...



Actual results:

The name displayed in the Network panel is actually "test" (missing "?" char)


Expected results:

The developer tool (Network panel) should have displayed the name "?test" in the "Params" sections
Component: Untriaged → Developer Tools: Netmonitor
Keywords: testcase
Status: UNCONFIRMED → NEW
Has STR: --- → yes
Ever confirmed: true
The 'first' question mark is used as a separator, and is not part of the query string, see also:
https://en.wikipedia.org/wiki/Query_string

So, what's the use case?

The following works as expected:

http://example.com?ok=12 

I am seeing on arg 'ok' in the Params side panel

Do you mean the following:
http://example.com??ok=12 

Honza
Flags: needinfo?(edznux)
You are testing with the "GET" method in your example.

Correct me if I'm wrong but POST request send the data in the body so it should not need a separator.
Flags: needinfo?(edznux)
Attached image IE11 vs Fx52.png — 

    
    

    
  
(In reply to edznux from comment #2)
> You are testing with the "GET" method in your example.
> 
> Correct me if I'm wrong but POST request send the data in the body so it
> should not need a separator.

See the comment 3.

    
    

    
  
OK, I see it now, thanks for the explanation!

Honza
Flags: qe-verify+
Keywords: good-first-bug
Whiteboard: [netmonitor-reserve]

    
    

    
  
Hi, I'd like to work on this bug.

    
    

    
  
Excellent, assigned to you!

Honza
Assignee: nobody → brennan.brisad

    
    

    
  

      
      
      
      

        Attached patch
          
          
        bug1344511.patch (obsolete)
        — 
          — Splinter Review
      

    


  
Attaching a patch for feedback.  I made a new test case but found later that I had to modify devtools/client/netmonitor/test/browser_net_complex-params.js also.  I think that modification is probably enough and that my test case is superfluous.  But I kept it in the patch for now and can remove it if you agree.

I also found a new bug when looking at the POST request which I filed here https://bugzilla.mozilla.org/show_bug.cgi?id=1350059

        Attachment #8850673 -
        Flags: feedback?(odvarko)

    
    

    
  
Comment on attachment 8850673 [details] [diff] [review]
bug1344511.patch

Review of attachment 8850673 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for the patch!

Looks good to me, just two things:

- Agree, I think the new test can be removed
- Please rebase on the new HEAD. We changed the directory structure a bit in bug 1350215

Honza

        Attachment #8850673 -
        Flags: feedback?(odvarko) → feedback-

    
    

    
  
Comment on attachment 8850673 [details] [diff] [review]
bug1344511.patch

Feedback is actually + ;-)

Honza

        Attachment #8850673 -
        Flags: feedback- → feedback+

    
    

    
  
Comment on attachment 8852640 [details] [diff] [review]
bug1344511.patch

Review of attachment 8852640 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for working on this!

Honza

        Attachment #8852640 -
        Flags: review?(odvarko) → review+

    
    

    
  
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/58dfedd4fb41
Do not remove leading '?' in form data shown in netmonitor. r=honza
Keywords: checkin-needed

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/58dfedd4fb41
Status: NEW → RESOLVED
Closed: 7 years ago

          status-firefox55:
          --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55

    
    

    
  
I have reproduced this issue on Firefox nightly according to (2017-03-04)

Fixing bug is verified on Latest Firefox Nightly--- Build ID:(20170429030208),User Agent:Mozilla/5.0 (Windows NT 6.1; rv:55.0) Gecko/20100101 Firefox/55.0

Tested OS-- Windows7 32bit
[testday-20170428]

    
    

    
  
Reproduced this issue on an Nightly build 54.0a1 from 2017-03-03.

This is also verified on 55 beta 3 (20170619141703) using Windows 10 x64, macOS 10.12.5 and Ubuntu 16.04 x64 LTS.
Status: RESOLVED → VERIFIED

          status-firefox55:
          fixed → verified
Flags: qe-verify+

    
  
Product: Firefox → DevTools

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: