Skip to content

The JsonPathDetector class does not handle empty objects correctly #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
tetrachromium opened this issue Jun 23, 2024 · 4 comments
Closed

Comments

@tetrachromium
Copy link
Contributor

The JsonPathDetector class handles JSON objects and arrays slightly differently, due to the need to recognize an object's property keys. To handle this, the JsonPathDetector state machine uses an additional "pending" state when processing objects. During the "pending" state, no path item has been pushed for properties of the object.

When the closing brace of an empty object is encountered, the state is still "pending" since no properties have been seen. The current implementation of the state machine currently assumes that a path item has been pushed. It therefore incorrectly pops the key of the enclosing object or array.

Consider the following JSON fragment:

{
  "outer": {
    "inner": {
    } // Should have path [ "outer", "inner" ], but has path [ "outer" ]
  }
}

When a JsonPathDetector object encounters the closing brace of the "inner" object, the path is just [ "outer", "inner" ] because no nested properties have been found. Ordinarily the key of the last nested property would be popped, but the "inner" key is popped instead. The JsonChunk is therefore incorrectly given an [ "outer" ] path. The paths of all subsequent JsonChunk objects will then be incorrect.

@tetrachromium
Copy link
Contributor Author

I have implemented a unit test and fix for this issue, but cannot push it due to insufficient access.

@tetrachromium
Copy link
Contributor Author

tetrachromium commented Jun 23, 2024

Unit test that illustrates the problem. This test will currently fail.

test("PathDetector handles empty object", async () => {
	const stream = serializeJsonValue({
		outer: {
			inner: {}
		}
	}).pipeThrough(new JsonPathDetector());

	expect(await streamToArray(stream)).toEqual([
		{ ...objectStart(), path: [] },
		{ ...stringStart(StringRole.KEY), path: [] },
		{ ...stringChunk("outer", StringRole.KEY), path: [] },
		{ ...stringEnd(StringRole.KEY), path: [] },
		{ ...colon(), path: [] },
		{ ...objectStart(), path: ["outer"] },
		{ ...stringStart(StringRole.KEY), path: ["outer"] },
		{ ...stringChunk("inner", StringRole.KEY), path: ["outer"] },
		{ ...stringEnd(StringRole.KEY), path: ["outer"] },
		{ ...colon(), path: ["outer"] },
		{ ...objectStart(), path: ["outer", "inner"] },
		{ ...objectEnd(), path: ["outer", "inner"] },
		{ ...objectEnd(), path: ["outer"] },
		{ ...objectEnd(), path: [] }
	]);
});

@tetrachromium
Copy link
Contributor Author

Pull request #2 has been created to address this issue.

@cdauth
Copy link
Owner

cdauth commented Jun 23, 2024

Brilliant, thanks for the report, the detailed explanation and the fix! I've released v1.2.1 containing the fix.

@cdauth cdauth closed this as completed Jun 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants