-
Notifications
You must be signed in to change notification settings - Fork 201
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
Link uri encoding, URL-escaping should be left alone inside the destination #598
Conversation
By the Rule "URL-escaping should be left alone inside the destination", Reimplement `normalizeLinkDestination` util function with splitting by URL escapings and concat them
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
I renewed Google CLA. It should work now |
Pull Request Test Coverage Report for Build 8335977223Details
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks so much for the fix! One suggestion
lib/src/util.dart
Outdated
// Refer: https://spec.commonmark.org/0.30/#example-502. | ||
|
||
final regex = RegExp('%[0-9A-Fa-f]{2}'); | ||
final matches = regex.allMatches(destination).toList(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this work of tracking the matches
and the splitIterator
can be simplified with splitMapJoin. Something like:
return destination.splitMapJoin(regex,
onMatch: (m) => m.input,
onNonMatch: (e) {
try {
e = Uri.decodeFull(e);
} catch (_) {}
return Uri.encodeFull(decodeHtmlCharacters(e));
},
);
WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't heard splitMapJoin
. Thank you for your suggestion. I refactored.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Slick!
Revisions updated by `dart tools/rev_sdk_deps.dart`. crypto (https://github.com/dart-lang/crypto/compare/f059196..69d13c9): 69d13c9 2024-03-21 Kevin Moore Switch sha512 to use fastpath with wasm (dart-archive/crypto#165) csslib (https://github.com/dart-lang/csslib/compare/b58e487..4216525): 4216525 2024-03-21 Devon Carew prep for publishing 1.0.1 (dart-archive/csslib#197) dartdoc (https://github.com/dart-lang/dartdoc/compare/7be9e24..79c1675): 79c16759 2024-03-25 Parker Lougheed Migrate client code to package:web (dart-lang/dartdoc#3610) 0b1c7fa4 2024-03-25 dependabot[bot] Bump actions/cache from 4.0.1 to 4.0.2 (dart-lang/dartdoc#3734) 9fe35ec5 2024-03-20 Sam Rawlins mustachio: Separate out the context stack LUB type calculation (dart-lang/dartdoc#3730) http (https://github.com/dart-lang/http/compare/5dfea72..7949d6f): 7949d6f 2024-03-25 Brian Quinlan cupertino_http: upgrade ffigen version (dart-lang/http#1159) 051482a 2024-03-22 Brian Quinlan Ready cupertino_http for release with WebSocket support (dart-lang/http#1158) 988b4d4 2024-03-20 Brian Quinlan Prepare package:cronet_http 1.2 for release (dart-lang/http#1157) 69f4eff 2024-03-20 Hossein Yousefi [cronet_http] Upgrade jni to 0.7.3 (dart-lang/http#1156) d8b1a9e 2024-03-19 Brian Quinlan Release `package:web_socket` 0.1.0 (dart-lang/http#1155) cfbc191 2024-03-19 Brian Quinlan Add a WebSocket implementation to package:cupertino_http (dart-lang/http#1153) markdown (https://github.com/dart-lang/markdown/compare/9c6b1af..8d07abc): 8d07abc 2024-03-19 MJ Studio Link uri encoding, URL-escaping should be left alone inside the destination (dart-lang/markdown#598) web (https://github.com/dart-lang/web/compare/4af904f..c522718): c522718 2024-03-20 Kevin Moore Update MDN documentation (dart-lang/web#213) f80dcab 2024-03-15 Srujan Gaddam Update pubspec description to be consistent with README (dart-lang/web#210) 27936c4 2024-03-15 Devon Carew Generate api docs for getters (dart-lang/web#207) 2f13cd5 2024-03-12 Devon Carew fix unresolved dartdoc links (dart-lang/web#200) 686827a 2024-03-12 Srujan Gaddam Remove reference to static interop and point to dart.dev page for JS interop (dart-lang/web#206) 9b7e29d 2024-03-12 Devon Carew Add a 'sourced from mdn docs' line to the MDN sourced dartdoc (dart-lang/web#198) 51e594b 2024-03-05 Srujan Gaddam Fix dictionary constructors to accept supertype members and create an empty object when there are no fields (dart-lang/web#197) Change-Id: Ic90c6f5a7e7d701746276031a8028cdfe76bc27a Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/359880 Reviewed-by: Konstantin Shcheglov <scheglov@google.com> Commit-Queue: Devon Carew <devoncarew@google.com>
Fix #597
By the Commonmark Spec URL-escaping should be left alone inside the destination, reimplement
normalizeLinkDestination
util function with splitting by URL Escapings and concatenating them again.We shouldn't use
decodeXXX
at initial string in any ways.This doesn't make problem with fixed issue #589. All tests previous exist also are passed.
Contribution guidelines:
dart format
.Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.