Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #531 - Title retrieving is failing with multiple use case #532

Merged

Conversation

ArthurHoaro
Copy link
Member

see #531 for details

Almost everything is now working, except:

@ArthurHoaro ArthurHoaro self-assigned this Apr 6, 2016
@ArthurHoaro ArthurHoaro added bug it's broken! in review labels Apr 6, 2016
@ArthurHoaro ArthurHoaro added this to the 0.7.0 milestone Apr 6, 2016
'ssl' => array(
'verify_peer' => false,
'verify_peer_name' => false,
'allow_self_signed' => true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure at all we should ignore SSL/TLS verification entirely...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the potential risk?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

interesting read: PHP Security - Transport Layer Security

SSL/TLS should be enforced by default, with a possibility for users to disable it (at their own risk)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't find a serious risk off the the top of my head, but bypassing security mechanisms tends to not end well. I agree that this should be optional.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not take the time to read @virtualtam paper yet, but I don't think an option is necessary here. We can just remove TLS bypass.

@ArthurHoaro
Copy link
Member Author

Update:

  • Better Accept-Language header.
  • Support of International Domain Names (requires php-intl).
  • Support xhtml charset definition:
<meta http-equiv="Content-Type" content="application/xhtml+xml;charset=iso-8859-1" />

$this->assertEquals($target, getAbsoluteUrl($origin, $target));
}

public function tmpTest()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

temporarily empty? :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I like my empty test methods. :(

@ArthurHoaro ArthurHoaro force-pushed the hotfix/title-retrieve-the-return branch 2 times, most recently from d32cb07 to 6e7db3b Compare May 3, 2016 17:47
@ArthurHoaro ArthurHoaro force-pushed the hotfix/title-retrieve-the-return branch from 6e7db3b to ce7b0b6 Compare May 3, 2016 17:51
@ArthurHoaro ArthurHoaro merged commit 47be060 into shaarli:master May 3, 2016
@ArthurHoaro ArthurHoaro deleted the hotfix/title-retrieve-the-return branch May 3, 2016 17:54
* @param string $url initial URL to reach.
* @param int $redirectionLimit max redirection follow..
* @param string $url initial URL to reach.
* @param int $redirectionLimit max redirection follow..

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 dots at the end?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug it's broken! security
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants