linaro-license-protection

Merge lp://qastaging/~gesha/linaro-license-protection/fix-https-and-chrome into lp://qastaging/~linaro-automation/linaro-license-protection/trunk

  1. fix-https-and-chrome
  2. Merge into trunk
Proposed by Georgy Redkozubov
Status: Merged
Merged at revision: 63
Proposed branch: lp://qastaging/~gesha/linaro-license-protection/fix-https-and-chrome
Merge into: lp://qastaging/~linaro-automation/linaro-license-protection/trunk
Diff against target: 221 lines (+84/-36)
3 files modified
.htaccess (+18/-5)
licenses/license.php (+64/-29)
licenses/ste.html (+2/-2)
To merge this branch: bzr merge lp://qastaging/~gesha/linaro-license-protection/fix-https-and-chrome
Reviewer Review Type Date Requested Status
James Tunnicliffe (community) Approve
Paul Sokolovsky Approve
Review via email: mp+103482@code.qastaging.launchpad.net

Description of the change

This branch adds https processing and fixes cookies handling.

To post a comment you must log in.
lp://qastaging/~gesha/linaro-license-protection/fix-https-and-chrome updated
67. By Georgy Redkozubov

Updated filename handling

68. By Georgy Redkozubov

Added symlinks support

Revision history for this message
Paul Sokolovsky (pfalcon) wrote :

I was here and eyeballed it for typos, etc.

Not sure I understand functionality enough to comment on that. One thing I could point to is:

+function check_file($fn)
70 +{
71 + if (is_file($fn) or is_link($fn)) {

if (file_exists($fn) and check_file($fn)) { // Requested download is file

Symlink is assumed to be a file, but it can point to a dir too.

review: Approve
Revision history for this message
James Tunnicliffe (dooferlad) wrote :

This looks fine to me. I assume it passes the tests :-)

On the subject of tests, I don't think we have one for the condition that Paul spotted (simlink to something that isn't a file). I don't think it is worth holding up releasing this since I don't think we simlink to directories at the moment on snapshots or releases, but it is worth filing a bug and getting it fixed when you have the time.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches