mipearson / webpack-rails

Integrate webpack with your Ruby on Rails application
MIT License
543 stars 82 forks source link

Fixes SSL certificate verification #49

Closed ares closed 7 years ago

mipearson commented 8 years ago

This will break things for existing users depending on this behaviour.

That said - we're < 1.0, and I've never been comfortable with VERIFY_NONE anyway.

Let me think about it.

ares commented 8 years ago

I started with verify none as default but then realized that implicit should be secure. I think major bump would be ok, maybe even minor since it's really a security fix. With update instructions to achieve the previous behavior it should be fine.

Anyway, leaving up to you.

Odesláno pomocí AquaMail pro Android http://www.aqua-mail.com

Dne 16. srpna 2016 13:22:56 mipearson notifications@github.com napsal:

This will break things for existing users depending on this behaviour.

That said - we're < 1.0, and I've never been comfortable with VERIFY_NONE anyway.

Let me think about it.

You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: https://github.com/mipearson/webpack-rails/pull/49#issuecomment-240074883

mipearson commented 8 years ago

Ah - if you're the contributor who originally added the VERIFY_NONE then I'm happy to merge your fix.

On Wed, Aug 17, 2016 at 12:44 AM, Marek Hulán notifications@github.com wrote:

I started with verify none as default but then realized that implicit should be secure. I think major bump would be ok, maybe even minor since it's really a security fix. With update instructions to achieve the previous behavior it should be fine.

Anyway, leaving up to you.

Odesláno pomocí AquaMail pro Android http://www.aqua-mail.com

Dne 16. srpna 2016 13:22:56 mipearson notifications@github.com napsal:

This will break things for existing users depending on this behaviour.

That said - we're < 1.0, and I've never been comfortable with VERIFY_NONE anyway.

Let me think about it.

You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: https://github.com/mipearson/webpack-rails/pull/49# issuecomment-240074883

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/mipearson/webpack-rails/pull/49#issuecomment-240124265, or mute the thread https://github.com/notifications/unsubscribe-auth/AAF8JXYYGYbnty9GE9PiIXZ_oGXiB6flks5qgczqgaJpZM4JlRyu .

Michael Pearson

ares commented 8 years ago

No, I'm not :-) I meant I started this PR with verify none, but then I changed my mind and sent this version.

Odesláno pomocí AquaMail pro Android http://www.aqua-mail.com

Dne 16. srpna 2016 11:52:16 PM mipearson notifications@github.com napsal:

Ah - if you're the contributor who originally added the VERIFY_NONE then I'm happy to merge your fix.

On Wed, Aug 17, 2016 at 12:44 AM, Marek Hulán notifications@github.com wrote:

I started with verify none as default but then realized that implicit should be secure. I think major bump would be ok, maybe even minor since it's really a security fix. With update instructions to achieve the previous behavior it should be fine.

Anyway, leaving up to you.

Odesláno pomocí AquaMail pro Android http://www.aqua-mail.com

Dne 16. srpna 2016 13:22:56 mipearson notifications@github.com napsal:

This will break things for existing users depending on this behaviour.

That said - we're < 1.0, and I've never been comfortable with VERIFY_NONE anyway.

Let me think about it.

You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: https://github.com/mipearson/webpack-rails/pull/49# issuecomment-240074883

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/mipearson/webpack-rails/pull/49#issuecomment-240124265, or mute the thread https://github.com/notifications/unsubscribe-auth/AAF8JXYYGYbnty9GE9PiIXZ_oGXiB6flks5qgczqgaJpZM4JlRyu .

Michael Pearson

You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: https://github.com/mipearson/webpack-rails/pull/49#issuecomment-240250781

ares commented 8 years ago

any chance you have thought about it?

ares commented 8 years ago

Thanks @mipearson, I updated the PR.

ares commented 8 years ago

Anything else I could do to get this in?

mipearson commented 8 years ago

I'm going to be bringing this in to an API-breaking 1.0-pre branch shortly.

The delay on this (and other) pull requests is that I've been cautious to merge anything that could break functionality for existing users.

ares commented 8 years ago

Sure, that I can definitely understand. Thanks for the update.

mipearson commented 7 years ago

Merged in 0.9.10, but disabled by default

ares commented 7 years ago

Thanks you :+1: