Opened 2 years ago

Closed 23 months ago

#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 2 years ago by fdupont

PS: please classify this ticket...

comment:2 Changed 2 years 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 2 years 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 23 months ago by marcin

  • Owner changed from UnAssigned to marcin

comment:5 Changed 23 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 23 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.