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

fix: JSON block string encoding #843

Merged
merged 13 commits into from
Jul 15, 2024

Conversation

mattjohnsonpint
Copy link
Contributor

Fixes #839

Note, I considered using gjson.AppendJSONString for better performance, but it currently doesn't have an option to disable escaping of HTML characters - which I think would be undesirable here. See tidwall/gjson#362

@mattjohnsonpint

This comment was marked as resolved.

@mattjohnsonpint mattjohnsonpint marked this pull request as ready for review July 2, 2024 18:05
@mattjohnsonpint

This comment was marked as resolved.

@mattjohnsonpint

This comment was marked as resolved.

@mattjohnsonpint mattjohnsonpint changed the title Fix JSON string encoding fix: JSON block string encoding Jul 2, 2024
@mattjohnsonpint mattjohnsonpint marked this pull request as draft July 2, 2024 19:14
@mattjohnsonpint
Copy link
Contributor Author

mattjohnsonpint commented Jul 2, 2024

And now, after checking the relevant part of the GraphQL spec, I also realize that block quote indentation is not being handled correctly.

For example:

query {
  foo(bar: 
"""
abc
def
""")
}

and

query {
  foo(bar: 
      """
      abc
      def
      """)
}

... should be treated equivalently. Currently, the latter includes extraneous whitespace on the second line.

Update forthcoming...

@mattjohnsonpint
Copy link
Contributor Author

Block indentation has been fixed, with passing tests.

BTW, I tried figuring out how to fix it in the lexer, but ran into problems because the resulting tokens are always just start/end pointers back to the original raw text. For the indentation parsing algorithm to work correctly, it has to be able to exclude data from the middle of a character sequence. So instead, I just wrote functions to find the original raw block string, and then parsed it according to the spec. Hope that works for y'all.

@mattjohnsonpint mattjohnsonpint marked this pull request as ready for review July 3, 2024 01:36
@devsergiy devsergiy merged commit 699eb81 into wundergraph:master Jul 15, 2024
14 checks passed
@StarpTech
Copy link
Collaborator

@mattjohnsonpint thank you!

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

Successfully merging this pull request may close these issues.

Error in datasource when querying using triple quotes syntax containing quotes or newlines
3 participants