Skip to content

Sanitize output from handleJsonSuccess / handleJsonError / invalidServerResponse #3130

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

Open
jonasfj opened this issue Sep 20, 2021 · 2 comments
Labels
type-enhancement A request for a change that isn't a bug

Comments

@jonasfj
Copy link
Member

jonasfj commented Sep 20, 2021

In http.dart we have few function that dumps output from the package repository server to terminal:

pub/lib/src/http.dart

Lines 304 to 346 in 570cb28

void handleJsonSuccess(http.Response response) {
var parsed = parseJsonResponse(response);
if (parsed['success'] is! Map ||
!parsed['success'].containsKey('message') ||
parsed['success']['message'] is! String) {
invalidServerResponse(response);
}
log.message(log.green(parsed['success']['message']));
}
/// Handles an unsuccessful JSON-formatted response from pub.dartlang.org.
///
/// These responses are expected to be of the form `{"error": {"message": "some
/// message"}}`. If the format is correct, the message will be raised as an
/// error; otherwise an [invalidServerResponse] error will be raised.
void handleJsonError(http.Response response) {
var errorMap = parseJsonResponse(response);
if (errorMap['error'] is! Map ||
!errorMap['error'].containsKey('message') ||
errorMap['error']['message'] is! String) {
invalidServerResponse(response);
}
fail(log.red(errorMap['error']['message']));
}
/// Parses a response body, assuming it's JSON-formatted.
///
/// Throws a user-friendly error if the response body is invalid JSON, or if
/// it's not a map.
Map parseJsonResponse(http.Response response) {
Object value;
try {
value = jsonDecode(response.body);
} on FormatException {
invalidServerResponse(response);
}
if (value is! Map) invalidServerResponse(response);
return value;
}
/// Throws an error describing an invalid response from the server.
void invalidServerResponse(http.Response response) =>
fail(log.red('Invalid server response:\n${response.body}'));

It would probably be wise to apply some sanitizing to this output, similar to what @themisir did for message="..." in www-authenticate.

IMO, we should break message sanitizing logic into a utility function and use when printing output from a server.

In particular I think it's unreasonable to allow servers to print ANSI escape codes, and such... maybe a few newlines, but not too many. And not too long messages.

I haven't check if there is anything weird you can do here, I'm just imagining there could be...

@jonasfj jonasfj added the type-enhancement A request for a change that isn't a bug label Sep 20, 2021
@jonasfj
Copy link
Member Author

jonasfj commented Sep 20, 2021

Playing around with this a bit it seems ANSI escape codes can be encoded JSON:

import 'dart:convert';

void main() {
  print('# Print test in bold:');
  print('\u001b[1m test \u001b[22m');
  print('');

  print('# Encode as JSON:');
  final j = json.encode({'msg': '\u001b[1m test \u001b[22m'});
  print(j);
  print('');

  print('# Print decoded JSON:');
  print(json.decode(j));
  print('');

  print('# Encode as utf8 and print decoded utf-8:');
  final b = utf8.encode(j);
  print(utf8.decode(b));
  print('');

  print('# Encode as utf8 and print decoded utf-8 and JSON:');
  print(json.decode(utf8.decode(b)));
}

We probably shouldn't allow package repositories to send ANSI escape codes to the terminal.

@jonasfj
Copy link
Member Author

jonasfj commented Nov 8, 2022

When looking at this we should also clearify when to use:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

1 participant