zendframework / zend-diactoros

PSR-7 HTTP Message implementation
BSD 3-Clause "New" or "Revised" License
1.56k stars 152 forks source link

Header error with Docker container usage #350

Closed adamculp closed 5 years ago

adamculp commented 5 years ago

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

Steps to reproduce the issue

Expected results

Expected to receive typical output from Expressive skeleton application output.

Actual results

Zend\Diactoros\Exception\InvalidArgumentException raised in file /var/www/vendor/zendframework/zend-diactoros/src/HeaderSecurity.php line 145: Message: "<address>Apache/2.4.25 (Debian) Server at localhost Port 8080</address>" is not valid header value
michalbundyra commented 5 years ago

I think it is the same problem as #347 and now the solution is in #349.

adamculp commented 5 years ago

It is possibly the same. But since I experienced the error in a container, but no errors on a local dev environment. So I added this new ticket.

maiorano84 commented 5 years ago

Bumping this ticket. Also experiencing a similar problem on a Docker installation.

EDIT:

As a point of clarification, a local Valet installation will work just fine with this package. When using it in a docker container however, argv is actually passed in my case as one of the headers (the contents of which is simply an empty array).

I'm not sure if the setting is stemming from either my nginx:alpine or php:7.2-fpm images, but I'm thinking it's the latter. I could chalk this issue up as an environment problem that this package isn't intended to handle, but the one thing I'm curious about is that this particular issue isn't present in the latest version 1 build.

I could potentially patch this in my own application by providing a cleaned copy of the $_SERVER superglobal to the fromGlobals factory method, but if you have any other ideas or patches in mind, keep us in the loop.

Ocramius commented 5 years ago

@maiorano84 tests required

maiorano84 commented 5 years ago

@Ocramius I've provided a minimal example and the steps to reproduce here:

https://github.com/maiorano84/zend-diactoros-bug-env

If you're using PHPStorm, you should be able to wire up XDebug with the provided configuration and set up a breakpoint on index.php to see what I'm talking about.

geerteltink commented 5 years ago

There are some more issues here. I just encountered some issues with docker containers.

It took me a while to trace this back to zend-diactoros because the zend-expressive error handler did not kick at the point. Right now I created a hack in index.php:

    unset($_SERVER['affinity:container']);
    foreach ($_SERVER as $key => $value) {
        if ($value === []) {
            unset ($_SERVER[$key]);
        }
    }

Wouldn't it be better to filter out invalid keys and values instead of throwing exceptions?

maiorano84 commented 5 years ago

@xtreamwayz The affinity:container index doesn't appear to be a Docker standard. Can you put together a minimal environment of your own so that this can be verified? See my example above for details.

geerteltink commented 5 years ago

@maiorano84 This is my docker-compose.yml:

version: "3"
services:

  web:
    image: xtreamwayz/nginx
    container_name: web
    ports:
      - "80:80"
    volumes:
      - ./public:/app/public

  php:
    image: xtreamwayz/php:7.1-fpm
    container_name: php
    volumes:
      - .:/app
      - /app/.git
      - /app/.idea
      - /app/data/mysql
      - /app/node_modules

  db:
    image: xtreamwayz/mysql
    container_name: db
    ports:
      - "3306:3306"
    volumes:
      - ./res/mysql-initdb:/docker-entrypoint-initdb.d:ro
      - ./data/mysql:/var/lib/mysql:rw

  redis:
    image: redis:3-alpine
    container_name: redis

  mailhog:
    image: mailhog/mailhog
    container_name: mailhog
    ports:
      - "8025:8025"

PHP container is based on php:7.1-fpm-alpine, nginx is based on nginx:stable-alpine

michalbundyra commented 5 years ago

I have just checked it and it's broken because of #344, if you use version 2.0.1 it works perfectly on docker. As I mentioned before the fix is already in #349 and it should fix all the issues.

Hope someone can release it soon!

geerteltink commented 5 years ago

Affinity

In some cases, the placement of a container must be relative to other containers. Swarm lets you define those relationships through affinities.

The following will run two Redis servers, while guaranteeing they don’t get scheduled on the same machine:

docker run -d --name redis1 -e ‘affinity:container!=redis’ redis docker run -d --name redis2 -e ‘affinity:container!=redis’ redis

Affinities are automatically generated when the relationship between containers is implied. Containers that use options such as –link, –volumes-from and –net=container: get co-scheduled on the same host.

Source: https://blog.docker.com/2015/02/scaling-docker-with-swarm/

I also found this: https://github.com/docker/compose/issues/1598

Compose v1.3.0+ adds a set of affinity:container environment variables inside containers when those are recreated (second time a docker-compose up -d command is issued).

So unless added manually affinity:container added by docker-compose up -d.

From the docker-compose changelog.

Improved compatibility with swarm by only setting a container affinity to the previous instance of a services' container when the service uses an anonymous container volume. Previously the affinity was always set on all containers.

I think this is the related code in docker-compose.

https://github.com/docker/compose/blob/f9061720b53acdd3d634b191e2d29b087de54efa/compose/service.py#L918-L922

maiorano84 commented 5 years ago

@xtreamwayz That clarifies a lot, thank you. I've never seen it in any of my environments before, but it could very well be that my local versions are different or I've just been blind all this time. Apologies for the mixup.

weierophinney commented 5 years ago

Fixed with versions 2.0.3 and 2.1.1.