Skip to content

Commit

Permalink
fix(dotnet): crash when TEMP contains certain unicode characters (#1913)
Browse files Browse the repository at this point in the history
In certain environments, the temporary directory path may contain
unicode characters. If those are not properly encoded as UTF-8 when sent
to the `@jsii/kernel` process, a failure will occur when trying to
access such paths.

This change contains fixes for two possible sources of problems in such
situations:

1. In `@jsii/runtime`, the `SyncStdio` class was a little too eager in
   turning it's `Buffer` to UTF-8 strings, as it could have only a part
   of a multi-byte character sequence (the conversion would then fail
   or result in corruption). Instead, it now looks for `\n` directly on
   the `Buffer`, and only performs string conversion once one has been
   found.
2. In the .NET Runtime, the `NodeProcess` class would spawn using the
   `System.Diagnostic.Process` class without specifying input and output
   encodings, which by default are the `System.Console` encoding. If
   that happens to not be `UTF-8`, the result could be unreliable.
   Instead, the encodings are now forced to `System.Text.Encoding.UTF8`
   for all three pipes of the child process (`Input`, `Output` and
   `Error`).

Reproducing the conditions for the failure reported in aws/aws-cdk#7456 is
somewhat difficult, especially in the context of CI/CD. This makes it difficult
to validate those fixes actually deliver on their promises.
  • Loading branch information
RomainMuller committed Aug 18, 2020
1 parent f89ec65 commit 8f31b1a
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 17 deletions.
Expand Up @@ -11,4 +11,4 @@ internal interface INodeProcess : IDisposable

TextReader StandardError { get; }
}
}
}
Expand Up @@ -4,6 +4,7 @@
using System.IO;
using System.Reflection;
using System.Runtime.Versioning;
using System.Text;
using Microsoft.Extensions.Logging;

namespace Amazon.JSII.Runtime.Services
Expand All @@ -26,15 +27,19 @@ public NodeProcess(IJsiiRuntimeProvider jsiiRuntimeProvider, ILoggerFactory logg
if (string.IsNullOrWhiteSpace(runtimePath))
runtimePath = jsiiRuntimeProvider.JsiiRuntimePath;

var utf8 = new UTF8Encoding(false /* no BOM */);
_process = new Process
{
StartInfo = new ProcessStartInfo
{
FileName = "node",
Arguments = "--max-old-space-size=4096 " + runtimePath,
Arguments = $"--max-old-space-size=4096 {runtimePath}",
RedirectStandardInput = true,
StandardInputEncoding = utf8,
RedirectStandardOutput = true,
RedirectStandardError = true
StandardOutputEncoding = utf8,
RedirectStandardError = true,
StandardErrorEncoding = utf8
}
};

Expand Down
24 changes: 10 additions & 14 deletions packages/@jsii/runtime/lib/sync-stdio.ts
Expand Up @@ -5,9 +5,10 @@ const STDOUT_FD = 1;
const STDERR_FD = 2;
const INPUT_BUFFER_SIZE = 1024 * 1024; // not related to max line length

const EMPTY_BUFFER = Buffer.alloc(0);

export class SyncStdio {
private readonly inputQueue = new Array<string>();
private currentLine = '';
private bufferedData = EMPTY_BUFFER;

public writeErrorLine(line: string) {
this.writeBuffer(Buffer.from(`${line}\n`), STDERR_FD);
Expand All @@ -19,24 +20,16 @@ export class SyncStdio {

public readLine(): string | undefined {
const buff = Buffer.alloc(INPUT_BUFFER_SIZE);
while (this.inputQueue.length === 0) {
while (!this.bufferedData.includes('\n', 0, 'utf-8')) {
try {
const read = fs.readSync(STDIN_FD, buff, 0, buff.length, null);

if (read === 0) {
return undefined;
}

const str = buff.slice(0, read).toString();

for (const ch of str) {
if (ch === '\n') {
this.inputQueue.push(this.currentLine);
this.currentLine = '';
} else {
this.currentLine += ch;
}
}
const newData = buff.slice(0, read);
this.bufferedData = Buffer.concat([this.bufferedData, newData]);
} catch (e) {
// HACK: node may set O_NONBLOCK on it's STDIN depending on what kind of input it is made
// of (see https://github.com/nodejs/help/issues/2663). When STDIN has O_NONBLOCK, calls may
Expand All @@ -52,7 +45,10 @@ export class SyncStdio {
}
}

const next = this.inputQueue.shift();
const newLinePos = this.bufferedData.indexOf('\n', 0, 'utf-8');
const next = this.bufferedData.slice(0, newLinePos).toString('utf-8');
this.bufferedData = this.bufferedData.slice(newLinePos + 1);

return next;
}

Expand Down

0 comments on commit 8f31b1a

Please sign in to comment.