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

Allow query string parameters without values #636

Merged
merged 1 commit into from
May 23, 2016
Merged

Conversation

moozzyk
Copy link
Contributor

@moozzyk moozzyk commented May 19, 2016

Addresses #624

@dnfclas
Copy link

dnfclas commented May 19, 2016

Hi @moozzyk, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by .NET Foundation and real humans are currently evaluating your PR.

TTYL, DNFBOT;

@khellang
Copy link
Contributor

LGTM :shipit:

@moozzyk
Copy link
Contributor Author

moozzyk commented May 23, 2016

/cc @Tratcher

[InlineData("?q", "q")]
[InlineData("?q&", "q")]
[InlineData("?q1=abc&q2", "q2")]
public void QueryCollection_TEST(string queryString, string emptyParam)
Copy link
Member

Choose a reason for hiding this comment

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

Test names need work. Component_Scenario

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My bad - forgot to rename.

@moozzyk moozzyk force-pushed the pawelka/empty-params branch from 169312a to ca0a77e Compare May 23, 2016 22:26
@moozzyk
Copy link
Contributor Author

moozzyk commented May 23, 2016

🆙📅 - For the tests I used the same convention as used by the test that already existed.

{
var features = new FeatureCollection();
var request = new HttpRequestFeature();
request.QueryString = "&&";
Copy link
Member

Choose a reason for hiding this comment

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

there should be a leading ? on here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did not put the leading '?' based on https://github.com/aspnet/HttpAbstractions/blob/dev/src/Microsoft.AspNetCore.WebUtilities/QueryHelpers.cs#L126 (and the code that follows this). I don't feel strongly about this so I can add '?' if you prefer to have it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will do this:

        [Theory]
        [InlineData("?&&")]
        [InlineData("?&")]
        [InlineData("&&")]

@Tratcher
Copy link
Member

:shipit:

@moozzyk moozzyk force-pushed the pawelka/empty-params branch from ca0a77e to 675df65 Compare May 23, 2016 22:41
@moozzyk moozzyk force-pushed the pawelka/empty-params branch from 675df65 to e854d3a Compare May 23, 2016 22:41
@moozzyk moozzyk merged commit e854d3a into dev May 23, 2016
@moozzyk moozzyk deleted the pawelka/empty-params branch May 23, 2016 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.

4 participants