(core) Fix issue with sandboxUtil where %s in message would get interpreted wrongly

Summary:
- Also converted sandboxUtil to typescript.
- The issue with %s manifested when a Python traceback contained "%s" in the
  string; in that case the object with log metadata (e.g. docId) would
  confusingly replace %s as if it were part of the message from Python.

Test Plan: Added a test case for the fix.

Reviewers: alexmojaki

Reviewed By: alexmojaki

Differential Revision: https://phab.getgrist.com/D3486
This commit is contained in:
Dmitry S 2022-06-14 02:49:59 -04:00
parent 983638a5c5
commit b57a211741
2 changed files with 17 additions and 26 deletions

View File

@ -1,26 +1,18 @@
/** /**
* Various utilities and constants for communicating with the python sandbox. * Various utilities and constants for communicating with the python sandbox.
*/ */
import * as MemBuffer from 'app/common/MemBuffer';
import * as log from 'app/server/lib/log';
var MemBuffer = require('app/common/MemBuffer');
var log = require('./log');
/** /**
* SandboxError is an error type for reporting errors forwarded from the sandbox. * SandboxError is an error type for reporting errors forwarded from the sandbox.
*/ */
function SandboxError(message) { export class SandboxError extends Error {
// Poorly documented node feature, required to make the derived error keep a proper stack trace. constructor(message: string) {
Error.captureStackTrace(this, this.constructor); super("[Sandbox] " + (message || 'Python reported an error'));
this.name = 'SandboxError'; }
this.message = "[Sandbox] " + (message || 'Python reported an error');
} }
SandboxError.prototype = new Error();
// We need to set the .constructor property for Error.captureStackTrace to work correctly.
SandboxError.prototype.constructor = SandboxError;
exports.SandboxError = SandboxError;
/** /**
@ -30,9 +22,9 @@ exports.SandboxError = SandboxError;
* DATA = data must be a value to return to a call from the other side * DATA = data must be a value to return to a call from the other side
* EXC = data must be an exception to return to a call from the other side * EXC = data must be an exception to return to a call from the other side
*/ */
exports.CALL = null; export const CALL = null;
exports.DATA = true; export const DATA = true;
exports.EXC = false; export const EXC = false;
/** /**
@ -40,19 +32,18 @@ exports.EXC = false;
* The logged output is line-oriented, so that the prefix is only inserted at the start of a line. * The logged output is line-oriented, so that the prefix is only inserted at the start of a line.
* Binary data is encoded as with JSON.stringify. * Binary data is encoded as with JSON.stringify.
*/ */
function makeLinePrefixer(prefix, logMeta) { export function makeLinePrefixer(prefix: string, logMeta: object) {
var partial = ''; let partial = '';
return data => { return (data: Uint8Array) => {
partial += MemBuffer.arrayToString(data); partial += MemBuffer.arrayToString(data);
var newline; let newline;
while ((newline = partial.indexOf("\n")) !== -1) { while ((newline = partial.indexOf("\n")) !== -1) {
var line = partial.slice(0, newline); const line = partial.slice(0, newline);
partial = partial.slice(newline + 1); partial = partial.slice(newline + 1);
// Escape some parts of the string by serializing it to JSON (without the quotes). // Escape some parts of the string by serializing it to JSON (without the quotes).
log.rawInfo(prefix + JSON.stringify(line).slice(1, -1).replace(/\\"/g, '"') log.origLog('info', "%s%s", prefix,
.replace(/\\\\/g, '\\'), JSON.stringify(line).slice(1, -1).replace(/\\"/g, '"').replace(/\\\\/g, '\\'),
logMeta); logMeta);
} }
}; };
} }
exports.makeLinePrefixer = makeLinePrefixer;

View File

@ -126,7 +126,7 @@ export function setTmpLogLevel(level: string, optCaptureFunc?: (level: string, m
* captures those at minLevel and higher. Returns a promise for the array of "level: message" * captures those at minLevel and higher. Returns a promise for the array of "level: message"
* strings. These may be tested using testUtils.assertMatchArray(). Callback may return a promise. * strings. These may be tested using testUtils.assertMatchArray(). Callback may return a promise.
*/ */
export async function captureLog(minLevel: string, callback: any) { export async function captureLog(minLevel: string, callback: () => void|Promise<void>): Promise<string[]> {
const messages: string[] = []; const messages: string[] = [];
const prevLogLevel = log.transports.file.level; const prevLogLevel = log.transports.file.level;
const name = _.uniqueId('CaptureLog'); const name = _.uniqueId('CaptureLog');