You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2022/03/22 12:59:34 UTC
[GitHub] [apisix] kwanhur opened a new pull request #6686: feat(plugin-redirect): set redirect server port when enable http_to_https
kwanhur opened a new pull request #6686:
URL: https://github.com/apache/apisix/pull/6686
### Description
<!-- Please include a summary of the change and which issue is fixed. -->
Support set redirect server port, instead of only the well-known port `443`.
1. When enable APISIX default https server, its port 9443, redirect to 9443 could meet the expectation.
So support to set the port with config item `ret_port`, it only works on enable `http_to_https` to `true`.
- prefix `ret_` consistent with `ret_code`.
- `ret_port` ranges `1-65535`.
2. Fixed example `Test Plugin` code demo.
<!-- Please also include relevant motivation and context. -->
Fixes #4400
### Checklist
- [x] I have explained the need for this PR and the problem it solves
- [x] I have explained the changes or the new features added to this PR
- [x] I have added tests corresponding to this change
- [x] I have updated the documentation to reflect this change
- [x] I have verified that this change is backward compatible (If not, please discuss on the [APISIX mailing list](https://github.com/apache/apisix/tree/master#community) first)
<!--
Note
1. Mark the PR as draft until it's ready to be reviewed.
2. Always add/update tests for any changes unless you have a good reason.
3. Always update the documentation to reflect the changes made in the PR.
4. Make a new commit to resolve conversations instead of `push -f`.
5. To resolve merge conflicts, merge master instead of rebasing.
6. Use "request review" to notify the reviewer after making changes.
7. Only a reviewer can mark a conversation as resolved.
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] kwanhur commented on pull request #6686: feat(plugin-redirect): set redirect server port when enable http_to_https
Posted by GitBox <gi...@apache.org>.
kwanhur commented on pull request #6686:
URL: https://github.com/apache/apisix/pull/6686#issuecomment-1075886567
From plugin redirect docs, [rule 1](https://github.com/apache/apisix/blob/master/docs/en/latest/plugins/redirect.md)
> Only one of `http_to_https`, `uri` or `regex_uri` can be specified.
1. Under `uri` mode, it had supported to specify redirect port or host, like [test case 15-16](https://github.com/apache/apisix/blob/master/t/plugin/redirect.t#L346-L394).
- specify redirect port just update plugin's uri like(the other field is also the same solution)
```shell
"plugins": {
"redirect": {
"uri": "https://$host:9443$request_uri",
"ret_code": 301
}
}
```
2. Under `http_to_https` mode, extend `uri` field, it seems to break the rule 1 and bring a big change.
To extend the other field, what about new attribute `location`?
- If empty, its default value `https://$host$request_uri`.
- If not empty, it goes to the same with `uri`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] leslie-tsang commented on pull request #6686: feat(plugin-redirect): set redirect server port when enable http_to_https
Posted by GitBox <gi...@apache.org>.
leslie-tsang commented on pull request #6686:
URL: https://github.com/apache/apisix/pull/6686#issuecomment-1076574561
> I have verified that this change is backward compatible (If not, please discuss on the [APISIX mailing list](https://github.com/apache/apisix/tree/master#community) first)
Looks like a breaking change here, discussion in mail list first in needed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tokers commented on pull request #6686: feat(plugin-redirect): set redirect server port when enable http_to_https
Posted by GitBox <gi...@apache.org>.
tokers commented on pull request #6686:
URL: https://github.com/apache/apisix/pull/6686#issuecomment-1075798013
@kwanhur IMHO specify a port is really hacky, what if users say we need to customize the host in the Location, instead of the one passed from the client? Do we need to add another `ret_host` field?
Why not extend the `uri` field, let it also be effective in the `http_to_https` mode, so that we can custom the Location.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tokers commented on pull request #6686: feat(plugin-redirect): set redirect server port when enable http_to_https
Posted by GitBox <gi...@apache.org>.
tokers commented on pull request #6686:
URL: https://github.com/apache/apisix/pull/6686#issuecomment-1076131697
> Under http_to_https mode, extend uri field, it seems to break the rule 1 and bring a big change.
It's a big change but not broken? Let's hear more voices from the community :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org