#5513 closed defect (fixed)

feed JSON does not handle strings

Reported by: fdupont Owned by: fdupont
Priority: low Milestone: Kea1.4
Component: Unclassified Version: git
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DHCP Feature Depending on Ticket:
Estimated Difficulty: 0 Add Hours to Ticket: 1
Total Hours: 0 Internal?: no


A bad news, feed JSON code is easily fooled by a string in the input, e.g.:

// This test verifies that a string is correctly handled                        
TEST_F(JSONFeedTest, string) {
    std::string json = "{ \"braces\": \"}}}}\" }";
    ElementPtr expected = Element::createMap();
    expected->set("braces", Element::create("}}}}"));
    EXPECT_EQ(*expected, *Element::fromJSON(json));
    testRead(json, expected);

fails at the last line:

[ RUN      ] JSONFeedTest.string
unknown file: Failure
C++ exception with description "Unterminated string in <wire>:0:15" thrown in the test body.
[  FAILED  ] JSONFeedTest.string (1 ms)

A good news: ECMA 404, i.e. the official JSON specification, provides in figure 5 the automaton of a JSON string which can be greatly simplified (IMHO 2 new states at needed).

An extra point: correctness here means to not reject legal JSON, not to reject illegal JSON. BTW fromJSON already fits in this definition as it is known to accept illegal extra commas...


Change History (6)

comment:1 Changed 21 months ago by fdupont

PS: please classify this ticket...

comment:2 Changed 21 months ago by fdupont

  • Milestone changed from Kea-proposed to Kea1.4
  • Owner set to fdupont
  • Priority changed from medium to low
  • Status changed from new to accepted

1.4 low by 2018-01-18 conf call.

comment:3 Changed 21 months ago by fdupont

  • Add Hours to Ticket changed from 0 to 1
  • Owner changed from fdupont to UnAssigned
  • Status changed from accepted to reviewing

As I expected it was enough to add two new states. Ready for review.

comment:4 Changed 20 months ago by marcin

  • Owner changed from UnAssigned to marcin

comment:5 Changed 20 months ago by marcin

  • Owner changed from marcin to fdupont

Reviewed up to the commit f76ce0132f121bab1e4fd2685dd5e34e65a8b683

The code changes are good. But this ticket requires a ChangeLog entry.

comment:6 Changed 20 months ago by fdupont

  • Resolution set to fixed
  • Status changed from reviewing to closed

Merged with a ChangeLog entry. Closing.

Note: See TracTickets for help on using tickets.