Skip to content

Commit

Permalink
Fix a potential exponential backtracking issue when parsing quoted he…
Browse files Browse the repository at this point in the history
…aders (#1434)
  • Loading branch information
brianquinlan authored Dec 19, 2024
1 parent 3ce9451 commit dada989
Show file tree
Hide file tree
Showing 5 changed files with 97 additions and 3 deletions.
7 changes: 7 additions & 0 deletions pkgs/http_parser/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
## 4.1.2-wip

* Fixed a bug where parsing quoted header values could require a regex to
backtrack
* Fixed a bug where quoted header values containing escaped quotes would not
be correctly parsed.

## 4.1.1

* Move to `dart-lang/http` monorepo.
Expand Down
9 changes: 7 additions & 2 deletions pkgs/http_parser/lib/src/scan.dart
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,12 @@ final token = RegExp(r'[^()<>@,;:"\\/[\]?={} \t\x00-\x1F\x7F]+');
final _lws = RegExp(r'(?:\r\n)?[ \t]+');

/// A quoted string.
final _quotedString = RegExp(r'"(?:[^"\x00-\x1F\x7F]|\\.)*"');
///
/// [RFC-2616 2.2](https://datatracker.ietf.org/doc/html/rfc2616#section-2.2)
/// defines the `quoted-string` production. This expression is identical to
/// the RFC definition expect that, in this regex, `qdtext` is not allowed to
/// contain `"\"`.
final _quotedString = RegExp(r'"(?:[^"\x00-\x1F\x7F\\]|\\.)*"');

/// A quoted pair.
final _quotedPair = RegExp(r'\\(.)');
Expand Down Expand Up @@ -54,7 +59,7 @@ List<T> parseList<T>(StringScanner scanner, T Function() parseElement) {
return result;
}

/// Parses a single quoted string, and returns its contents.
/// Parses a double quoted string, and returns its contents.
///
/// If [name] is passed, it's used to describe the expected value if it's not
/// found.
Expand Down
2 changes: 1 addition & 1 deletion pkgs/http_parser/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: http_parser
version: 4.1.1
version: 4.1.2-wip
description: >-
A platform-independent package for parsing and serializing HTTP formats.
repository: https://github.com/dart-lang/http/tree/master/pkgs/http_parser
Expand Down
8 changes: 8 additions & 0 deletions pkgs/http_parser/test/authentication_challenge_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,14 @@ void _singleChallengeTests(
equals({'realm': 'fblthp, foo=bar', 'baz': 'qux'}));
});

test('parses quoted string parameters with surrounding spaces', () {
final challenge =
parseChallenge('scheme realm= "fblthp, foo=bar" , baz= "qux" ');
expect(challenge.scheme, equals('scheme'));
expect(challenge.parameters,
equals({'realm': 'fblthp, foo=bar', 'baz': 'qux'}));
});

test('normalizes the case of the scheme', () {
final challenge = parseChallenge('ScHeMe realm=fblthp');
expect(challenge.scheme, equals('scheme'));
Expand Down
74 changes: 74 additions & 0 deletions pkgs/http_parser/test/scan_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

import 'package:http_parser/src/scan.dart';
import 'package:string_scanner/string_scanner.dart';
import 'package:test/test.dart';

void main() {
group('expectQuotedString', () {
test('no open quote', () {
final scanner = StringScanner('test"');
expect(
() => expectQuotedString(scanner),
throwsA(isA<StringScannerException>()
.having((e) => e.offset, 'offset', 0)
.having((e) => e.message, 'message', 'expected quoted string.')
.having((e) => e.source, 'source', 'test"')));
expect(scanner.isDone, isFalse);
expect(scanner.lastMatch, null);
expect(scanner.position, 0);
});

test('no close quote', () {
final scanner = StringScanner('"test');
expect(
() => expectQuotedString(scanner),
throwsA(isA<StringScannerException>()
.having((e) => e.offset, 'offset', 0)
.having((e) => e.message, 'message', 'expected quoted string.')
.having((e) => e.source, 'source', '"test')));
expect(scanner.isDone, isFalse);
expect(scanner.lastMatch, null);
expect(scanner.position, 0);
});

test('simple quoted', () {
final scanner = StringScanner('"test"');
expect(expectQuotedString(scanner), 'test');
expect(scanner.isDone, isTrue);
expect(scanner.lastMatch?.group(0), '"test"');
expect(scanner.position, 6);
});

test(r'escaped \', () {
final scanner = StringScanner(r'"escaped: \\"');
expect(expectQuotedString(scanner), r'escaped: \');
expect(scanner.isDone, isTrue);
expect(scanner.lastMatch?.group(0), r'"escaped: \\"');
expect(scanner.position, 13);
});

test(r'bare \', () {
final scanner = StringScanner(r'"bare: \"');
expect(
() => expectQuotedString(scanner),
throwsA(isA<StringScannerException>()
.having((e) => e.offset, 'offset', 0)
.having((e) => e.message, 'message', 'expected quoted string.')
.having((e) => e.source, 'source', r'"bare: \"')));
expect(scanner.isDone, isFalse);
expect(scanner.lastMatch, null);
expect(scanner.position, 0);
});

test(r'escaped "', () {
final scanner = StringScanner(r'"escaped: \""');
expect(expectQuotedString(scanner), r'escaped: "');
expect(scanner.isDone, isTrue);
expect(scanner.lastMatch?.group(0), r'"escaped: \""');
expect(scanner.position, 13);
});
});
}

0 comments on commit dada989

Please sign in to comment.