laminas / laminas-view

Flexible view layer supporting and providing multiple view layers, helpers, and more
https://docs.laminas.dev/laminas-view/
BSD 3-Clause "New" or "Revised" License
74 stars 45 forks source link

[WIP] Broaden HTTPS Check #1

Open weierophinney opened 4 years ago

weierophinney commented 4 years ago

Because:

Provide a narrative description of what you are trying to accomplish:


Originally posted by @jbh at https://github.com/zendframework/zend-view/pull/201

weierophinney commented 4 years ago

Forgot to mention that this will resolve #194.


Originally posted by @jbh at https://github.com/zendframework/zend-view/pull/201#issuecomment-553980600

weierophinney commented 4 years ago

All checks failed? Ouch.


Originally posted by @jbh at https://github.com/zendframework/zend-view/pull/201#issuecomment-553982342

weierophinney commented 4 years ago

@jbh

It is common for end users to use ports other than 443 for HTTPS.

"common" – I do not think so; and if it were so, then something more must be change in the class and in some other components too. (e.g. zend-uri, zend-diactoros)

Mature systems, like IBM i, like to uppercase values

Sounds conclusive. We need unit tests to cover these changes. Thanks in advance!

All checks failed? Ouch.

Right, see the results on Travis: https://travis-ci.org/zendframework/zend-view/jobs/611967352#L439-L447


Originally posted by @froschdesign at https://github.com/zendframework/zend-view/pull/201#issuecomment-554098748

weierophinney commented 4 years ago

@froschdesign I work with many enterprise agencies, and they have plenty of internal, and even some external applications that they use HTTPS on different ports for. They would be pretty offended to hear that isn't common... This is a deal breaker for them when they try to use Apigility.

I'm trying to find a decent way to handle the tests. Thanks for the input, and pointing me to the proper error in Travis.

Edited to add a question If the maintainers insist I do not remove the 443 check, which I can agree is non-standard (not uncommon, though) what do they suggest I do to support applications built with HTTPS on the non-standard port?


Originally posted by @jbh at https://github.com/zendframework/zend-view/pull/201#issuecomment-554099919

weierophinney commented 4 years ago

@froschdesign and @jbh —

The way we handle it in Diactoros (per the PSR-7 specification) is to omit the port when it corresponds to the default port for the scheme used. So, for HTTP, you omit :80, and for HTTPS, you omit :443.

The IETF says that you can use the port in all cases, but suggests that it should be omitted when it is the default, which is why we went that route for Diactoros.

That's all that needs to happen here.


Originally posted by @weierophinney at https://github.com/zendframework/zend-view/pull/201#issuecomment-554105760