limeditor / chromedevtools

Automatically exported from code.google.com/p/chromedevtools
BSD 3-Clause "New" or "Revised" License
0 stars 0 forks source link

protocolparser dynamicimpl TypeHandler closedNameSet checking #79

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
http://code.google.com/p/chromedevtools/source/browse/trunk/plugins/org.chromium
.sdk/src-dynamic-impl/parser/org/chromium/sdk/internal/protocolparser/dynamicimp
l/TypeHandler.java#122

closedNameSet checking:

     // current version
     if (closedNameSet != null) {
        for (Object fieldNameObject : jsonProperties.keySet()) {
          if (!closedNameSet.contains(fieldNameObject)) {
            throw new JsonProtocolParseException("Unexpected field " + fieldNameObject);
          }
        }
      }

     // there should be checking also if set is empty because is always != null ?
     if (closedNameSet != null && closedNameSet.size() > 0) {
        for (Object fieldNameObject : jsonProperties.keySet()) {
          if (!closedNameSet.contains(fieldNameObject)) {
            throw new JsonProtocolParseException("Unexpected field " + fieldNameObject);
          }
        }
      }

Original issue reported on code.google.com by dragosla...@gmail.com on 24 Nov 2012 at 5:50

GoogleCodeExporter commented 8 years ago
Could you please explain why this change is necessary?  Why empty closedNameSet 
must have special meaning? Is there any scenario that demonstrates this bug?

Original comment by peter.ry...@gmail.com on 24 Nov 2012 at 11:46

GoogleCodeExporter commented 8 years ago
It tried something like code below and error occurs on parsing headers 
name/value set.
But it make sense if closedNameSet is size 0 it shouldn't throw error?

// data
{"method":"Network.requestWillBeSent", 
"params":{"requestId":"13933.204","frameId":"13933.1","loaderId":"13933.60","doc
umentURL":"http://triviaaddict.blogspot.com/2012/11/ingress-amazing-playing-fact
s.html?m=1","request":{"url":"http://triviaaddict.blogspot.com/2012/11/ingress-a
mazing-playing-facts.html?m=1","method":"GET","headers":{"User-Agent":"Mozilla/5
.0 (Linux; Android 4.0.4; HTC) AppleWebKit/535.19 (KHTML, like Gecko) 
Chrome/18.0.1025.166 Mobile 
Safari/535.19","Accept":"text/html,application/xhtml+xml,application/xml;q=0.9,*
/*;q=0.8","Cache-Control":"max-age=0"}},"timestamp":1353416325.303887,"initiator
":{"type":"other"},"stackTrace":[]}}

// code
JSONParser parser = new JSONParser();
org.json.simple.JSONObject jo = (org.json.simple.JSONObject) parser.parse(line);
org.json.simple.JSONObject params = 
(org.json.simple.JSONObject)jo.get("params");

WipProtocolParser wipParser = WipParserAccess.get();
RequestWillBeSentEventData ed = 
wipParser.parseNetworkRequestWillBeSentEventData(params);

Original comment by dragosla...@gmail.com on 25 Nov 2012 at 9:48

GoogleCodeExporter commented 8 years ago
No, the empty closed name set is a valid case, it means that no properties are 
allowed.

You talk about Network.Headers type. It is described as an object with any 
properties in 
http://svn.webkit.org/repository/webkit/trunk/Source/WebCore/inspector/Inspector
.json
This means that the closedNameSet must be null for it, indicating that the 
property set is open.

Original comment by peter.ry...@gmail.com on 25 Nov 2012 at 4:20

GoogleCodeExporter commented 8 years ago
A quick fix for you could also be disabling validation:
DynamicParserImpl constructor has a boolean parameter that enables validation. 
You can make sure that it is called with false value.

Original comment by peter.ry...@gmail.com on 25 Nov 2012 at 4:22

GoogleCodeExporter commented 8 years ago

Original comment by peter.ry...@gmail.com on 25 Nov 2012 at 7:03

GoogleCodeExporter commented 8 years ago
The patch is ready
https://codereview.chromium.org/11421067/

Original comment by peter.ry...@gmail.com on 25 Nov 2012 at 7:11

GoogleCodeExporter commented 8 years ago
Disabling validation doesn't feel like proper solution.
So feel free to take time and make it right.
I really don't use code yet and I find out that there is no server side of wip 
protocol. Or I am missing something? 
I need code that would act as server and generate json object from java 
objects. Similar to PonyDebugger app.

Can you please confirm this my assumptions and give me some directions.

Original comment by dragosla...@gmail.com on 26 Nov 2012 at 5:51

GoogleCodeExporter commented 8 years ago
WIP is a client-side implementation for Chrome/WebKit debugging protocol. 
Google Chrome/Chromium browser is our server side.

See http://code.google.com/p/chromedevtools/wiki/WIP

As far as I understand, PonyDebugger emulates Chrome and implements server side 
of its protocol for Network domain. This project does not work with this part 
of protocol (Network domain) yet. Eventually Network support should be 
developed here.

If you need to access PonyDebugger or Chrome Network communication in Java, you 
probably could successfully reuse some code from this project (possibly 
contributing it back). Please, contact me on the mailing list if that's what 
you are interested in.

Original comment by peter.ry...@gmail.com on 26 Nov 2012 at 9:10

GoogleCodeExporter commented 8 years ago
Fixed in HEAD

Original comment by peter.ry...@gmail.com on 26 Nov 2012 at 9:11