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

Updater does not serialize carriage return line breaks (\r\n) correctly #517

Closed
angelo-v opened this issue Oct 15, 2021 · 4 comments · Fixed by #516
Closed

Updater does not serialize carriage return line breaks (\r\n) correctly #517

angelo-v opened this issue Oct 15, 2021 · 4 comments · Fixed by #516

Comments

@angelo-v
Copy link
Contributor

If a literal contains \r\n the carriage return is serialized to a real line break in the resulting SPARQL Update query, which leads to invalid syntax. This is demonstrated via a unit test in 40a50b8#diff-15043c9b2c959a81026ffd28eae448814aa6d5b02c2a6e3f23e09488ad6b11b5R54

Resulting query:

INSERT DATA { <https://pod.example/test/foo#subject> <https://pod.example/test/foo#predicate> "literal
\nvalue" .
 }
@angelo-v
Copy link
Contributor Author

Problem seems to originate from Literal.toNT which replaces \n but not \r

@timbl
Copy link
Member

timbl commented Oct 15, 2021

What else should be included? \t ?
What defines that list?
Maybe the N3 string serialization which IIRC originally copied or pointed to python.

@angelo-v
Copy link
Contributor Author

Indeed there may be more to consider

[9] STRING_LITERAL_QUOTE ::= '"' ([^#x22#x5C#xA#xD] | ECHAR | UCHAR)* '"'

see https://www.w3.org/TR/n-triples/#grammar-production-STRING_LITERAL_QUOTE
and https://stackoverflow.com/a/40834612/758813

I will write some test cases

@angelo-v
Copy link
Contributor Author

so according to the spec (link in my last comment) #x22 ("), #x5C (), #x0A (\n) and #xD (\r) are disallowed and need to be replaced

from those only carriage return was missing, which I now added in #516

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 a pull request may close this issue.

2 participants