evocode / lemonstand-patch

MIT License
1 stars 3 forks source link

Patch Fails: trailing whitespace. #2

Closed TuzBot closed 8 years ago

TuzBot commented 8 years ago

Hi @patrickheeney

First of all I would just like to take this opportunity to thank you for taking the time to make the patch and get it out there as quick as possible.

I uploaded the patch to SSH'd to the server and used the patch command patch -p1 < security-update.diff

Here were the results:

https://gist.github.com/TuzBot/36aedd4c396ee0d1b231ef58ed360bbd

After logging out and back in after apply the patch, I don't get the 'COOKIE_SALT is not set' warning either.

Once again thank you for your time and effort.

TuzBot commented 8 years ago

Hi @patrickheeney

This is a gist with errors when I apply the patch via GIT.

https://gist.github.com/TuzBot/1249218c10e696bcaf5122cec10fd5fb

patrickheeney commented 8 years ago

It looks like it failed to patch these files:

phproad/modules/phpr/classes/phpr_response.php
phproad/modules/phpr/classes/phpr_security.php
phproad/modules/phpr/classes/phpr_securityframework.php
phproad/system/initialize.php

On the first gist, is your Lemonstand store up to date before applying?

It looks like the second gist got further but has an issue with whitespace. I will need to look into this further to see why.

patrickheeney commented 8 years ago

You can also try this to turn the whitespace errors into warnings instead:

git apply --whitespace=warn security-update.diff

or have it fix them:

git apply --whitespace=fix security-update.diff
TuzBot commented 8 years ago

I did a 'check for updates' before I went ahead and did the server patch, to where LS was up to date, however after trying to apply the patch, and then goign back to 'check for updates' it now says "Module (core) version information not found in the database.", it seems as though it has accidentally removed the core version - version.

moondawg69 commented 8 years ago

Just tried the above, getting:

error: patch failed: phproad/modules/phpr/classes/phpr_response.php:255 error: phproad/modules/phpr/classes/phpr_response.php: patch does not apply error: patch failed: phproad/modules/phpr/classes/phpr_security.php:391 error: phproad/modules/phpr/classes/phpr_security.php: patch does not apply error: patch failed: phproad/system/initialize.php:110 error: phproad/system/initialize.php: patch does not apply

patrickheeney commented 8 years ago

@TuzBot I think that is normal. I had to create a version to apply the new LS update that doesn't exist with LS since they stopped creating updates. So it makes sense that new version no longer exists for them. Did the other commands work to apply the patch?

@moondawg69 Are there any more errors aside from that? Can you try both of the apply commands listed above to see if one pushes it through?

TuzBot commented 8 years ago

For your Ref: @patrickheeney

After trying the command via GIT

git apply --whitespace=fix security-update.diff

https://gist.github.com/TuzBot/339bac0b88979d338c9beccccb75a22c

moondawg69 commented 8 years ago

Yes, seems to have worked now:

security-update.diff:197: trailing whitespace.

security-update.diff:225: trailing whitespace.

security-update.diff:270: trailing whitespace.

security-update.diff:284: trailing whitespace. Added a consistency check (discovered by Taylor Hornby in his security-update.diff:294: trailing whitespace. Due to downstream errors, the OpenSSL removal now belongs in version warning: squelched 136 whitespace errors warning: 136 lines applied after fixing whitespace errors.

patrickheeney commented 8 years ago

@TuzBot @moondawg69 Thank you. Looks like it applied for both of you. Can you continue with the other steps to see if the patch is successful?

TuzBot commented 8 years ago

@patrickheeney after applying the patch with git apply --whitespace=fix security-update.diff then pushing it to the server I get permission error at /modules/core/updates/jda823j2.php

TuzBot commented 8 years ago

Another note for you @patrickheeney when going in to add a version for the core module, it seems to have updated all module version to NULL

patrickheeney commented 8 years ago

@TuzBot can you paste your error? It may be a server configuration error in how you are deploying your code. Git or the deploy process is trying to create a file that the permissions are not allowing.

Where are you seeing module version to null? Or perhaps the LS update process set it to null when that version doesn't exist? This was designed to distributed via the LS update process and is now being done manually so may need to re-think this.

moondawg69 commented 8 years ago

Ok, after uploading all to the server, logging back in (I got the COOKIE_SALT is not set. But just tried again) - all seems well, I checked for the keys file, that is there etc. Just did a test order, seems ok.

So thank you so very much Patrick, a true jump.

patrickheeney commented 8 years ago

@moondawg69 You have to login twice as part of the process. Sounds like you are patched!

TuzBot commented 8 years ago

@patrickheeney yes, it seems pushing those files up destroyed the phproad folder, and the core module folder, I was unable to delete them via SFTP, so had to delete via SSH and re-upload, seems I do have permission issues somewhere.

TuzBot commented 8 years ago

Everything has been pushed to the server ok now @patrickheeney, however logging in or out never got the COOKIE_SALT message, but the keys.php file exists with all the correct details inside?

This also apply for the first gist I posted:

https://gist.github.com/TuzBot/36aedd4c396ee0d1b231ef58ed360bbd

patrickheeney commented 8 years ago

Once config/keys.php gets created you shouldn't receive the COOKIE_SALT message anymore. During one of your other steps it must have created it.

TuzBot commented 8 years ago

Thank @patrickheeney, the fact some parts failed but at some point the config/keys.php was created, would this now be classed as patched?

patrickheeney commented 8 years ago

As long as config/keys.php exists and has a long string of random characters for COOKIE_SALT and the other files were correctly patched like they appear in the diff, then you should be fully patched.

patrickheeney commented 8 years ago

Closing this as we seemed to have resolve all the issues discussed. I also put together a troubleshoot guide for these common issues.