Skip to content
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

flatbuffers::Parser requires kTokenEof at end of Json #7584

Closed
alexames opened this issue Oct 14, 2022 · 2 comments
Closed

flatbuffers::Parser requires kTokenEof at end of Json #7584

alexames opened this issue Oct 14, 2022 · 2 comments

Comments

@alexames
Copy link
Contributor

I have a use case where I want to parse a JSON string representing a flatbuffer that is to be compiled, but that json string is embedded in the middle of a larger string. In my ideal world, I would be able to pass the pointer to the start of the json to parser.ParseJson(...) to parse it, then have some function that would allow me to find out how many bytes were consumed so that I can step over the json substring from my own code.

Unfortunately this doesn't work out of the box because ParseJson(...) requires that the string its given always end in a kTokenEof token. One relatively straightforward way to deal with this would be to add an option to IDLOptions to allow embedded json strings, then make a minor change to the parser logic to just stop parsing when it reaches the end of the root table instead of continuing on, looking for that kTokenEof.

Figuring out how many bytes were parsed isn't that hard either. The parser already has a protected cursor pointer variable on it, so if I want to see how many bytes were consumed I can just do this hacky thing:

class ParserThatCanGetCursorLocation : public flatbuffer::Parser {
 public:
  const char* cursor() { return cursor_; }
};

But it would be nice if that functionality were officially exposed somehow too as a bytesConsumed getter or something.

I'm happy to put together a PR myself to add this functionality but I wanted to check here first to make sure this sounds like something reasonable to add.

@dbaileychess
Copy link
Collaborator

I would be supportive of something like this.

@alexames
Copy link
Contributor Author

alexames commented Nov 4, 2022

Fixed with #7620

@alexames alexames closed this as completed Nov 4, 2022
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