Skip to content

Implemented Cross-Origin-Resource-Policy response header#86

Merged
GaProgMan merged 3 commits intoGaProgMan:mainfrom
miguelcrpinto:feature/issue-76-Cross-Origin-Resource-Policy_not_supported
Jun 5, 2023
Merged

Implemented Cross-Origin-Resource-Policy response header#86
GaProgMan merged 3 commits intoGaProgMan:mainfrom
miguelcrpinto:feature/issue-76-Cross-Origin-Resource-Policy_not_supported

Conversation

@miguelcrpinto
Copy link
Copy Markdown
Contributor

Implemented Cross-Origin-Resource-Policy response header according to the description on issue #76

@GaProgMan
Copy link
Copy Markdown
Owner

Thank you for this. I'll take a look at it today.

Copy link
Copy Markdown
Collaborator

@jamie-taylor-rjj jamie-taylor-rjj left a comment

Choose a reason for hiding this comment

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

Fantastic work, thanks for this @miguelcrpinto. I've spotted a typo in one of the classes - likely a copy-paste error. Would you mind taking a look when you have the chance?

We also need a major version bump in order to release to NuGet, but I'm happy taking that as there are (currently) two places where the version number will need to change. I'll also need to add the new header to the changelog. Again, I'm happy to take that task, too.

Comment thread src/Models/CrossOriginResourcePolicy.cs Outdated
/// Requests from any Origin (both same-site and cross-site) can read the resource.
/// Browsers are using this policy when an CORP header is not specified.
/// </summary>
public const string CrossOriginValue = "same-origin";
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔧 I believe there is a typo in this string value. It should be "cross-origin".

It's an easy typo to make, too. So no worries 👍

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It was indeed a copy-paste issue! Fixed!

@miguelcrpinto
Copy link
Copy Markdown
Contributor Author

@jamie-taylor-rjj I also bumped the version and updated the changelog accordingly. Can you please check if I changed everything needed?

@jamie-taylor-rjj
Copy link
Copy Markdown
Collaborator

In case you were wondering, I was using this guide when choosing emoji for the PR comments. Looking at this new version of the PR now, live on Twitch.

Copy link
Copy Markdown
Collaborator

@jamie-taylor-rjj jamie-taylor-rjj left a comment

Choose a reason for hiding this comment

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

Fantastic work @miguelcrpinto. Thank you for your contribution.

@GaProgMan GaProgMan merged commit 433cbb7 into GaProgMan:main Jun 5, 2023
@jamie-taylor-rjj
Copy link
Copy Markdown
Collaborator

closes #76

@miguelcrpinto
Copy link
Copy Markdown
Contributor Author

Thanks! I managed to still watch a few minutes of the review and merge on twitch!

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.

3 participants