Fix attribute key encoding

[es-sayers]

Q	A
Is bugfix?	✔️
New feature?	❌
Breaks BC?	❌
Fixed issues

Attribute keys are currently not encoded. Since Tag implements NoEncodeStringableInterface, HTML characters may be output unintentionally.

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 100.00%. Comparing base (b2fa82e) to head (5e99f12). Report is 1 commits behind head on master.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #212 +/- ## =========================================== Coverage 100.00% 100.00% - Complexity 784 786 +2 =========================================== Files 86 86 Lines 2090 2097 +7 =========================================== + Hits 2090 2097 +7 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[samdark]

I think that these should be escaped/encoded for security reasons. It doesn't make sense if there are forbidden characters anyway.

[samdark]

@es-sayers would you please add a line to CHANGELOG?

[vjik]

I think that these should be escaped/encoded for security reasons. It doesn't make sense if there are forbidden characters anyway.

This is not escaped/encoded. This code change name to another (>hello → >hello). It's incorrect, browser don't unescape attribute names.

Sanitizing (remove denied symbols) is better for security. We can make new method Html::sanitizeAttributeName() for it.

[samdark]

browser don't unescape attribute names

That's false.

<form method="GET">
  <label id="first">
    Field
    <input type="text" name="&gt;hello" required />
  </label>
  <input type="submit" name="save" value="Save" />
</form>

Try yourself. Backend will get >hello as GET name.

[vjik]

<input type="text" name="&gt;hello" required />

This PR about attribute key, not attribute value.

[es-sayers]

Attribute names must consist of one or more characters other than controls, U+0020 SPACE, U+0022 ("), U+0027 ('), U+003E (>), U+002F (/), U+003D (=), and noncharacters.

Since attribute names can't have these characters, I think it's best to remove the vulnerability and have the developer work around it.

[es-sayers]

Is there any alternative behavior that would be better?

With encoding, a dev might try to have >custom="value" and receive >custom="value". Which is a valid attribute but not what they wanted.

With removing control characters, a dev might have attribute>1="value" and attribute>>1="value2" which would both return as attribute1="..."

I think it might be better to just fully remove the invalid attribute while still rendering the rest of the attribute string. I think this is the only way to limit unexpected behavior while also removing the security risk.

Other than that, throwing InvalidArgumentException might apply here, similarly to how it is applied in Range::outputTag().

    public function outputTag(string $tagName): self
    {
        if ($tagName === '') {
            throw new InvalidArgumentException('The output tag name it cannot be empty value.');
        }

        $new = clone $this;
        $new->outputTag = $tagName;
        return $new;
    }

[samdark]

@vjik thanks for pointing that out. @es-sayers throwing an exception might be a better option.