[Webkit-unassigned] [Bug 42976] Implement IDBKeyPath parser.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=42976


Jeremy Orlow <jorlow at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #62579|review?                     |review-
               Flag|                            |




--- Comment #2 from Jeremy Orlow <jorlow at chromium.org>  2010-07-26 09:39:10 PST ---
(From update of attachment 62579)
WebCore/storage/IndexedDatabaseRequest.idl:36
 +          // FIXME: IDBKeyPath is public only to allow testing.
FIXMEs should have an action...i.e. delete as soon as we can because it's only here for testing...fix elsewhere.

LayoutTests/storage/indexeddb/idb-keypath-expected.txt:17
 +  indexedDB.makeKeyPath('a[34][20].foo[2].bar')
maybe insert a newline via "debug('');" between test cases? (here and elsewhere)



LayoutTests/storage/indexeddb/script-tests/idb-keypath.js:9
 +      for (;keyPath.hasNext(); keyPath.next(), ++i) {
space before keyPath...but why not put the var i = 0 in there?



WebCore/storage/IDBKeyPath.cpp:50
 +      Token token;
What does this do?

WebCore/storage/IDBKeyPath.cpp:63
 +  void IDBKeyPath::next()
I guess this next interface stuff makes sense in terms of easily testing (without building a lot of infrastructure that's just going to go away) but for the real implementation, I don't see why we don't just give a pointer to the raw array + size check.  This stuff seems overly complicated.

WebCore/storage/IDBKeyPath.cpp:173
 +      return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z');
this is a subset of javascript identifiers




Do we really want to support array syntax within keyRanges?  Also, what do we do when we see malformed keyPaths?  Right now it seems we try our best to parse them, but I don't think this is the right behavior.  I think we should raise an exception on parse errors.  (The create function should return 0 and the caller would raise.)  All of this really needs to be specced out tho.

Haven't looked at the parser or tokenizer in detail yet.  To be honest, it feels a little heavyweight for our needs, but I'm guessing it'll pay off in the long run.  We'll probably want multiple eyes looking at it though...since it'd be one of the easier ways to introduce security issues, I'd think.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.