Skip to content

Conversation

@laggardkernel
Copy link
Contributor

@laggardkernel laggardkernel commented Jun 13, 2021

Break the loop to t avoid appending string headers into failures multiple times. Otherwise it causes duplication in CORS response text (not response header).

        elif requested_headers is not None:
            for header in [h.lower() for h in requested_headers.split(",")]:
                if header.strip() not in self.allow_headers:
                    failures.append("headers")
+                    break
        ...
        if failures:
            failure_text = "Disallowed CORS " + ", ".join(failures)
            return PlainTextResponse(failure_text, status_code=400, headers=headers)

@Kludex Kludex added the cors Cross-Origin Resource Sharing label Jun 13, 2021
@JayH5
Copy link
Contributor

JayH5 commented Jun 13, 2021

Thanks. Any way we can add a test for this?

@laggardkernel
Copy link
Contributor Author

laggardkernel commented Jun 13, 2021

It's such a trivial change, just break the loop to avoid appending a value into the same list multiple times. Do we really need a test for it? @JayH5

@laggardkernel
Copy link
Contributor Author

Anyway, I added a test and force pushed the commit.

Copy link
Contributor

@JayH5 JayH5 left a comment

Choose a reason for hiding this comment

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

It's such a trivial change, just break the loop to avoid appending a value into the same list multiple times. Do we really need a test for it?

I didn't completely understand this at first so just thought bug == test please 🙂. Probably wasn't super necessary but can't hurt, thanks.

@JayH5 JayH5 merged commit 15761fb into Kludex:master Jun 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cors Cross-Origin Resource Sharing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants