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.

Sign up for GitHub

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

Jump to bottom

fix conversion from wchar to char in LockingTextWriter.put #6474

Merged
merged 6 commits into from
Apr 29, 2018
Merged

fix conversion from wchar to char in LockingTextWriter.put #6474

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d5cc4c5
fix conversion from wchar to char in LockingTextWriter.put
aG0aep6G Apr 21, 2018
8611e45
destroy file_ before possibly throwing
aG0aep6G Apr 23, 2018
2ab81bc
add comment about the final throw
aG0aep6G Apr 23, 2018
bf1b4db
only store the high surrogate, and use `\0` to indicate its empty
aG0aep6G Apr 23, 2018
5fa8ba1
merge imports
aG0aep6G Apr 23, 2018
b771e17
only decode when there's a highSurrogate
aG0aep6G Apr 24, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
83 changes: 78 additions & 5 deletions std/stdio.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2831,6 +2831,15 @@ is empty, throws an `Exception`. In case of an I/O error throws

// the file's orientation (byte- or wide-oriented)
int orientation_;

// A buffer for when we need to transcode.
wchar highSurrogate = '\0'; // '\0' indicates empty
void highSurrogateShouldBeEmpty() @safe
{
import std.utf : UTFException;
if (highSurrogate != '\0')
throw new UTFException("unpaired surrogate UTF-16 value");
}
public:

this(ref File f) @trusted
Expand All @@ -2851,6 +2860,10 @@ is empty, throws an `Exception`. In case of an I/O error throws
{
if (p.handle) FUNLOCK(p.handle);
}
file_ = File.init;
/* Destroy file_ before possibly throwing. Else it wouldn't be
destroyed, and its reference count would be wrong. */
highSurrogateShouldBeEmpty();
}

this(this) @trusted
Expand Down Expand Up @@ -2907,6 +2920,7 @@ is empty, throws an `Exception`. In case of an I/O error throws
static if (c.sizeof == 1)
{
// simple char
highSurrogateShouldBeEmpty();
if (orientation_ <= 0) trustedFPUTC(c, handle_);
else trustedFPUTWC(c, handle_);
}
Expand All @@ -2918,14 +2932,27 @@ is empty, throws an `Exception`. In case of an I/O error throws
{
if (c <= 0x7F)
{
highSurrogateShouldBeEmpty();
trustedFPUTC(c, handle_);
}
else
else if (0xD800 <= c && c <= 0xDBFF) // high surrogate
{
char[4] buf;
immutable size = encode(buf, c);
foreach (i ; 0 .. size)
trustedFPUTC(buf[i], handle_);
highSurrogateShouldBeEmpty();
highSurrogate = c;
}
else // standalone or low surrogate
{
dchar d = c;
if (highSurrogate != '\0')
{
immutable wchar[2] rbuf = [highSurrogate, c];
d = rbuf[].front;
highSurrogate = 0;
}
char[4] wbuf;
immutable size = encode(wbuf, d);
foreach (i; 0 .. size)
trustedFPUTC(wbuf[i], handle_);
}
}
else
Expand All @@ -2937,6 +2964,7 @@ is empty, throws an `Exception`. In case of an I/O error throws
{
import std.utf : encode;

highSurrogateShouldBeEmpty();
if (orientation_ <= 0)
{
if (c <= 0x7F)
Expand Down Expand Up @@ -3419,6 +3447,51 @@ void main()
assert(File(deleteme).readln() == "日本語日本語日本語日本語############日本語");
}

@safe unittest // wchar -> char
{
static import std.file;
import std.exception : assertThrown;
import std.utf : UTFException;

auto deleteme = testFilename();
scope(exit) std.file.remove(deleteme);

{
auto writer = File(deleteme, "w").lockingTextWriter();
writer.put("\U0001F608"w);
}
assert(std.file.readText!string(deleteme) == "\U0001F608");

// Test invalid input: unpaired high surrogate
{
immutable wchar surr = "\U0001F608"w[0];
auto f = File(deleteme, "w");
assertThrown!UTFException(() {
auto writer = f.lockingTextWriter();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Took me a few minutes to understand that the first assertThrown above is testing the throw from the lockingTextWriter dtor. Can you put in a comment to that effect?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a comment at the end of the block.

writer.put('x');
writer.put(surr);
assertThrown!UTFException(writer.put(char('y')));
assertThrown!UTFException(writer.put(wchar('y')));
assertThrown!UTFException(writer.put(dchar('y')));
assertThrown!UTFException(writer.put(surr));
// First `surr` is still unpaired at this point. `writer` gets
// destroyed now, and the destructor throws a UTFException for
// the unpaired surrogate.
} ());
}
assert(std.file.readText!string(deleteme) == "x");

// Test invalid input: unpaired low surrogate
{
immutable wchar surr = "\U0001F608"w[1];
auto writer = File(deleteme, "w").lockingTextWriter();
assertThrown!UTFException(writer.put(surr));
writer.put('y');
assertThrown!UTFException(writer.put(surr));
}
assert(std.file.readText!string(deleteme) == "y");
}

@safe unittest
{
import std.exception : collectException;
Expand Down