-
Notifications
You must be signed in to change notification settings - Fork 32
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: response values encoding to support compatibility with XML when generating the results #404
Conversation
f9dcdf8
to
ff02fe6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Escaping the XML when it's already built and stored is too late.
Please do that in here instead
qti-sdk/src/qtism/data/storage/xml/Utils.php
Line 356 in e02baa6
return htmlspecialchars((string)$value, ENT_XML1, 'UTF-8'); |
Also, please consider applying something like this instead of the HTML-entity-based solution.
function isInCharacterRange(int $char): bool
{
return $char == 0x09
|| $char == 0x0A
|| $char == 0x0D
|| $char >= 0x20 && $char <= 0xDF77
|| $char >= 0xE000 && $char <= 0xFFFD
|| $char >= 0x10000 && $char <= 0x10FFFF;
}
function xmlSpecialChars(string $value): string
{
$result = '';
$last = 0;
$length = strlen($value);
$i = 0;
while ($i < $length) {
$r = mb_substr(substr($value, $i), 0, 1);
$width = strlen($r);
$i += $width;
switch ($r) {
case '"':
$esc = '"';
break;
case "'":
$esc = ''';
break;
case '&':
$esc = '&';
break;
case '<':
$esc = '<';
break;
case '>':
$esc = '>';
break;
case "\t":
$esc = '	';
break;
case "\n":
$esc = '
';
break;
case "\r":
$esc = '
';
break;
default:
if (!isInCharacterRange(mb_ord($r)) || (mb_ord($r) === 0xFFFD && $width === 1)) {
$esc = "\u{FFFD}";
break;
}
continue 2;
}
$result .= substr($value, $last, $i - $last - $width) . $esc;
$last = $i;
}
return $result . substr($value, $last);
}
The solution was copied from the Go sources: https://go.dev/src/encoding/xml/xml.go#L1916.
8dfdb0f
to
d93c7e0
Compare
Thank you for advice now, looks like a working well this part |
replaced to suggested solution |
|
||
public function testProcessSpecialCharsetWithoutError(): void | ||
{ | ||
$xml = ('<?xml version="1.0" encoding="UTF-8"?> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure you need the parenthesis here.
</itemResult> | ||
</assessmentResult> | ||
'); | ||
$this->assertNotNull(Utils::findExternalNamespaces(sprintf($xml, Utils::valueAsString("160\b\u{0008}")))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$this->assertNotNull(Utils::findExternalNamespaces(sprintf($xml, Utils::valueAsString("160\b\u{0008}")))); | |
$this->assertNotNull(Utils::findExternalNamespaces(sprintf($xml, Utils::valueAsString("160\u{0008}")))); |
000c35c
to
f43572f
Compare
f43572f
to
314a30a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- New code is covered by tests (if applicable)
- Tests are running successfully (old and new ones) on my local machine (if applicable)
- New code is respecting code style rules
- New code is respecting best practices
- New code is not subject to concurrency issues (if applicable)
- Feature is working correctly on my local machine (if applicable)
- Acceptance criteria are respected
- Pull request title and description are meaningful
- Pull request's target is not
master
Allow special HTML symbols TR-6347
Description
This PR decoded special HTML symbols in responses before transforming them into XML object
to avoid error
Note
These changes not been fully tested that's why it is a draft