Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Don't set X-Forwarded-SSL: ON when headers indicate SSL was OFF before. #1077

Merged
merged 3 commits into from Jan 4, 2014

Conversation

Projects
None yet
1 participant
Member

skinkie commented Jan 3, 2014

My guess is that this could be merged. The following behavior is changed:

When an incomming SSL connection has an X-Forwarded-SSL which is OFF
the X-Forwarded-SSL will never set to ON. The only way that X-Forwarded-SSL
can be ON:

  • there is a secure connection and no X-Forwarded-SSL header is present -> ON
  • there is a secure connection and X-Forwarded-SSL is ON -> ON

In all other cases X-Forwarded SSL will be set OFF:

  • there is no secure connection and no header X-Forward-SSL -> OFF
  • there is no secure connection and X-Forwarded-SSL is ON -> OFF

There is a potential optimisation to copy the existing off header.
For readibility I did not go in that direction.

Fix #1073

skinkie added some commits Jan 3, 2014

Don't set X-Forwarded-SSL: ON when headers indicate SSL was OFF before.
My guess is that this could be merged. The following behavior is changed:

When an incomming SSL connection has an X-Forwarded-SSL which is OFF
the X-Forwarded-SSL will never set to ON. The only way that X-Forwarded-SSL
can be ON:

 - there is a secure connection and no X-Forwarded-SSL header is present -> ON
 - there is a secure connection and X-Forwarded-SSL is ON -> ON

In all other cases X-Forwarded SSL will be set OFF:
 - there is no secure connection and no header X-Forward-SSL -> OFF
 - there is no secure connection and X-Forwarded-SSL is ON -> OFF

There is a potential optimisation to copy the existing off header.
For readibility I did not go in that direction.

Fix #1073
Wrong length.
This shows in my humble opinion that this type of matching is error prone.

skinkie added a commit that referenced this pull request Jan 4, 2014

Merge pull request #1077 from cherokee/proxy_xforwaredssl_proposal_1073
Don't set X-Forwarded-SSL: ON when headers indicate SSL was OFF before.

@skinkie skinkie merged commit 1c9b654 into master Jan 4, 2014

@skinkie skinkie deleted the proxy_xforwaredssl_proposal_1073 branch Jan 4, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment