Url Path Decoder #448
Url Path Decoder #448
Conversation
troydai
force-pushed
the
trdai/kes124
branch
2 times, most recently
from
3607cd4 to
60d50aa
Compare
| /// <summary> | ||
| /// Find the next % in the sequence. If % is not found, return the sequence length. | ||
| /// </summary> | ||
| private static int NextPercentage(int start, string source) |
| } | ||
| } | ||
|
|
||
| private static bool TryGetUnescapedAscii(char first, char second, out char result) |
There was a problem hiding this comment.
This name and purpose are out of sync. You don't fail if it's > 0x7F and you keep doing bitwise operation on the resulting char. How about changing it to TryGetUnescapedByte with an out byte?
There was a problem hiding this comment.
Will update
- Name
- Out a
byte
|
Ok, I make one significant update. I added a function to do inplace decoding to avoid allocating additional char array. |
| new DecoderExceptionFallback()); | ||
|
|
||
| /// <summary> | ||
| /// Unescape a url char array in place. Returns the length of the result. |
There was a problem hiding this comment.
use standard doc comment tags like <returns>
|
Ping @blowdart for security review. |
|
Yes, why is this here? This needs to be in CoreFx. |
|
Sure, this can be moved to corefx eventually. I'm wondering what's the process of adding this to corefx and how long does it take since I have four days to fix the corresponding Kestrel issue 124. May be we can do this parallel. I'll open an PR on corefx begin adding this there in the meantime let's add this here first to get by the rc1. |
|
Make this more internal-ish in this repo then, as opposed to making it public-ish. (I don't care about actual accessibility, but I want it to look like it's going away, e.g. in a |
|
Will do. |
Features: - Minimal memory allocation. - Support decode string (in the form of char array) in place - UTF8 verification - Skip unescapse '%2F'
| using System; | ||
| using System.Text; | ||
|
|
||
| namespace Microsoft.Extensions.WebEncoders.Internals |
|
Close this one for now. I'm aiming at handle this entirely in Kestrel by itself. |
Addressing: aspnet/KestrelHttpServer#124
/cc @Tratcher @blowdart @lodejard
Note:
/.%2Fwill be leave in the string.Follow up: