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

features: append cors plugin #1327

Merged
merged 14 commits into from
Mar 29, 2020
Merged

features: append cors plugin #1327

merged 14 commits into from
Mar 29, 2020

Conversation

ShiningRush
Copy link
Contributor

Summary

cors plugin

Full changelog

  • support base cors header
  • support multiple origin
  • support forcefully match origin and methods with wildcard

@ShiningRush ShiningRush changed the title Feat/cors features: append cors plugin Mar 23, 2020
@moonming
Copy link
Member

@Miss-you please take a look, thx

properties = {
allow_origins = {
description =
"you can use '*' to allow all origins when no credentials,"..
Copy link
Member

Choose a reason for hiding this comment

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

code style: please add a space credentials," ..

end

if not conf.allow_origins then
conf.allow_origins = "*"
Copy link
Member

Choose a reason for hiding this comment

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

we can set default value by jsonschema.

end

if not conf.allow_methods then
conf.allow_methods = "*"
Copy link
Member

Choose a reason for hiding this comment

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

ditto, please fix other similar issues

if conf.allow_origins == "**" then
conf.allow_origins = ngx.var.http_origin or '*'
end
if str_find(conf.allow_origins, ",") then
Copy link
Member

Choose a reason for hiding this comment

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

end
if str_find(conf.allow_origins, ",") then
local finded = false
for origin in str_gmatch(conf.allow_origins, "([^,]+)") do
Copy link
Member

Choose a reason for hiding this comment

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

ngx.re.gmatch is better performance

t/plugin/cors.t Show resolved Hide resolved
@membphis
Copy link
Member

@ShiningRush missing doc

Copy link
Member

@membphis membphis left a comment

Choose a reason for hiding this comment

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

please update the English version too


## 属性

- `allow_origins`: `可选`.允许跨域访问的Origin, 格式如:`scheme`://`host`:`port`, 比如: https://somehost.com:8081. 多个值使用 `,` 分割, `allow_credential` 为 `false` 时可以使用 `*` 来表示所有 Origin均允许通过。你也可以在启用了 `allow_credential` 后使用 `**` 强制允许所有Origin都通过,但请注意这样存在安全隐患。默认值为 `*`。
Copy link
Member

Choose a reason for hiding this comment

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

"可选. " -> "可选,"

Copy link
Member

Choose a reason for hiding this comment

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

Use Chinese commas

Copy link
Member

Choose a reason for hiding this comment

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

add space between English and Chinese

- `allow_credential`: 是否允许跨域访问的请求方携带凭据(如 Cookie 等), 默认值为: `false`。

## 如何启用
创建 `Route` 或 `Service` 对象,并配置 `cors` 插件。
Copy link
Member

Choose a reason for hiding this comment

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

add a blank line

```shell
curl http://127.0.0.1:9080/apisix/admin/routes/1 -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
{
"methods": ["GET"],
Copy link
Member

Choose a reason for hiding this comment

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

we can remove this line

```shell
$ curl http://127.0.0.1:9080/apisix/admin/routes/1 -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
{
"methods": ["GET"],
Copy link
Member

Choose a reason for hiding this comment

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

we can remove this line

@membphis
Copy link
Member

add a reference to doc:

https://github.com/apache/incubator-apisix#features

@ShiningRush
Copy link
Contributor Author

The ci show the ./doc/plugins/cors-cn.md has no license header, but it does have, hmm, what happened to it?

@membphis
Copy link
Member

you should revert them:

image

@moonming moonming merged commit c363ea4 into apache:master Mar 29, 2020
This was referenced Mar 30, 2020
SaberMaster pushed a commit to SaberMaster/incubator-apisix that referenced this pull request Jun 30, 2020
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