Skip to content
This repository was archived by the owner on Dec 18, 2018. It is now read-only.

Unescape string in memory entirely. #274

Merged
merged 1 commit into from
Oct 23, 2015
Merged

Unescape string in memory entirely. #274

merged 1 commit into from
Oct 23, 2015

Conversation

troydai
Copy link
Contributor

@troydai troydai commented Oct 21, 2015

Escape the path url.

Feature:

  • Operate completely in the memory pool without allocate memory
  • UTF-8 decode once only.

/cc @lodejard @Tratcher @halter73 @muratg

  • Add a put function to MemoryPoolIterator2 for saving byte correctly
  • Add UTF8 verification during the pass


namespace Microsoft.AspNet.Server.Kestrel.Http
{
public class Decoder
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UrlPathDecoder

@troydai troydai force-pushed the trdai/kestrel124.2 branch 2 times, most recently from 2d061a0 to c25f886 Compare October 22, 2015 05:03
if (byteCount > 3)
{
writer.Put((byte)byte4);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using these if-else structure to avoid allocate a byte array.

@troydai troydai changed the title [Prototype] Unescape string in memory, entirely. Unescape string in memory, entirely. Oct 22, 2015
@troydai troydai changed the title Unescape string in memory, entirely. Unescape string in memory entirely. Oct 22, 2015
@troydai troydai force-pushed the trdai/kestrel124.2 branch from c25f886 to b0a85a9 Compare October 22, 2015 05:27
@troydai
Copy link
Contributor Author

troydai commented Oct 22, 2015

Ping

{
var decodeReader = reader;

// If decoding process successes, the writer iterator will be moved
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

super nit: succeeds.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Will update.

@troydai
Copy link
Contributor Author

troydai commented Oct 22, 2015

Ping, updated.

[InlineData("%20"," ")]
[InlineData("%%32", "%2")]
[InlineData("%%20", "% ")]
[InlineData("%C0%A4%32", "%C0%A42")]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These mixed invalid&valid should be a separate test case.

@troydai
Copy link
Contributor Author

troydai commented Oct 23, 2015

Added iteration to fix a few bugs found in review. I'm adding test cases to cover them.

@troydai troydai force-pushed the trdai/kestrel124.2 branch 4 times, most recently from 9e1c054 to 33aa245 Compare October 23, 2015 17:34
@troydai
Copy link
Contributor Author

troydai commented Oct 23, 2015

@troydai
Copy link
Contributor Author

troydai commented Oct 23, 2015

Ping

var chFound = scan.Seek(' ', '?');
if (chFound == -1)

int chFound = -1;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ch? Character? Separator?

1. In place unescape;
1. UTF-8 verification;
2. MemoryPoolIterator2.Put
3. Tests
@troydai troydai force-pushed the trdai/kestrel124.2 branch from 94d0898 to 52f4fa9 Compare October 23, 2015 22:27
@Tratcher
Copy link
Member

:shipit:

@troydai
Copy link
Contributor Author

troydai commented Oct 23, 2015

all right. I'll wait a bit for Travis.

@troydai troydai merged commit 52f4fa9 into dev Oct 23, 2015
@troydai troydai deleted the trdai/kestrel124.2 branch October 23, 2015 22:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants