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

report-uri should not be encoded at all #64

Merged
merged 2 commits into from
Mar 26, 2023

Conversation

Firesphere
Copy link
Contributor

The report-uri should, as report-to, be unencoded.

If any encoding happens on those URLs, the endpoint is parsed as current.website/{encoded-report-endpoint}

The report-uri should, as report-to, be unencoded
@frederikbosch
Copy link

I agree. The currently merged PR does urlencode which is also resulting in an inappropriate/unusable report-uri. I do think the new lines should be replaced.

My solution for now is:

$builder = new class($defaultPolicies) extends CSPBuilder {
    protected function enc(string $piece, string $type = 'default'): string
    {
        if ($type === 'url' || \str_starts_with($piece, 'https://') || \str_starts_with($piece, 'http://')) {
            return str_replace(
                ["\r", "\n"],
                ['%0D', '%0A'],
                $piece
            );
        }

        return parent::enc($piece, $type);
    }
};

@paragonie-security paragonie-security merged commit 85fd7c7 into paragonie:master Mar 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants