Skip to content
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

Add support in BasicHtmlWebResponseObject for open-ended input tags #16193

Merged

Conversation

farmerau
Copy link
Contributor

@farmerau farmerau commented Oct 4, 2021

PR Summary

Per #16126, BasicHtmlWebResponseObject didn't fully support valid html for input fields. Specifically, for an Invoke-WebRequest that returns this valid html:

<html>
    <body>
        <input name="foo" value="bar">
    </body>
</html>

would have a null InputFields value.

To solve this, I've added an alternative construct to the regular expression (as recommended by the author of the issue) that requires neither a closing slash '/' nor a closing input tag ''.

Additionally, I've created Pester test cases to validate the outcome of the updated expression.

PR Context

(For reference #15973)

PR Checklist

@ghost ghost assigned TravisEz13 Oct 4, 2021
@ghost
Copy link

ghost commented Oct 4, 2021

CLA assistant check
All CLA requirements met.

@iSazonov iSazonov added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Oct 5, 2021
@@ -219,7 +219,7 @@ private static void EnsureHtmlParser()

if (s_inputFieldRegex == null)
{
s_inputFieldRegex = new Regex(@"<input\s+[^>]*(/>|>.*?</input>)",
s_inputFieldRegex = new Regex(@"<input\s+[^>]*(/>|>|>.*?</input>)",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
s_inputFieldRegex = new Regex(@"<input\s+[^>]*(/>|>|>.*?</input>)",
s_inputFieldRegex = new Regex(@"<input\s+[^>]*(/?>|>.*?</input>)",

I think this is a bit of an easier to read / simpler regex expression that should have the same result.

@farmerau farmerau force-pushed the support-self-closing-input-tags-html branch from bca62e6 to 337e349 Compare October 7, 2021 00:44
- Supports open-ended (no end slash) input tags, to match void element spec.
- Adds unit tests confirming other forms of input tags are unaffected.

Update unit tests and simplify regular expression

Leverage testcases
@farmerau farmerau force-pushed the support-self-closing-input-tags-html branch from 337e349 to 9e51abe Compare October 7, 2021 08:54
@farmerau
Copy link
Contributor Author

@iSazonov is there a next step for merging? I'm not an authorized member. From what I can tell, it should be okay to happen 1 business day after approval.

@iSazonov
Copy link
Collaborator

@farmerau We are waiting MSFT members to review. Since it is Regex they can find compliance issues.

/cc @daxian-dbw @TravisEz13 friendly ping.

@daxian-dbw daxian-dbw assigned daxian-dbw and unassigned TravisEz13 Nov 2, 2021
@daxian-dbw daxian-dbw merged commit 107fd7a into PowerShell:master Nov 2, 2021
@iSazonov
Copy link
Collaborator

iSazonov commented Nov 2, 2021

@farmerau Thanks for your contribution!

@ghost
Copy link

ghost commented Dec 16, 2021

🎉v7.3.0-preview.1 has been released which incorporates this pull request.:tada:

Handy links:

TrapGodBrim pushed a commit to TrapGodBrim/PowerShell that referenced this pull request Jan 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants