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

Dynamic Version Salt - NOT WORKING [v. 140605] #231

Closed
soukupl opened this issue Jul 1, 2014 · 10 comments
Closed

Dynamic Version Salt - NOT WORKING [v. 140605] #231

soukupl opened this issue Jul 1, 2014 · 10 comments
Milestone

Comments

@soukupl
Copy link

soukupl commented Jul 1, 2014

hi,
I have just updated QuickCache pro to version 140605 (from 140104) and discovered two things...

  1. "MD5 Version Salt" setting has been renamed to "Dynamic Version Salt"
  2. settings was NOT moved from MD5 salt to Dynamic salt :(
  3. "Dynamic Version Salt" can NOT be set and is ignored! I need to have "$_SESSION['user']['PersonalNumber']" there to distinguish more logged in users - we are using non-WP login as the site depends on external API data source and external login (there are no private data stored on the server except the cached ones)

I have downgraded back to 140104. Please, look at this issue. Looking forward for fixed version :)
BTW: QuickCache is GREAT plugin and our top choice :)

@raamdev
Copy link
Contributor

raamdev commented Jul 1, 2014

Hi @soukupl,

Can you tell me what you mean by ""Dynamic Version Salt" can NOT be set"? Are you receiving an error? Or are you added the version salt but Quick Cache appears to be ignoring what you've added?

Can you copy/paste exactly what you're adding to the Dynamic Version Salt field?

@soukupl
Copy link
Author

soukupl commented Jul 1, 2014

Hi @raamdev ,
I'm trying to add this

$_SESSION['user']['PersonalNumber']

I have tested it on several hosts with the same result... Value of "Dynamic Version Salt" is not saved. On older version, "MD5 Version Salt" was working with the same value.

I have created screencast from 3 hosts (2 with new version and one with older version).
link: https://www.youtube.com/watch?v=9Rx2zR5cinI

@raamdev
Copy link
Contributor

raamdev commented Jul 1, 2014

@soukupl Thank you for the additional information and the video!

I can confirm that I'm seeing the same behavior on my end. It looks like this might be a bug.

@jaswsinc When I try to save $_SESSION['user']['PersonalNumber'] as the Dynamic Version Salt, I get a PHP Notice:

Notice: Undefined variable: _SESSION in .../wp-content/advanced-cache.php on line 160

It shouldn't be necessary for the dynamic version salt to always be present, should it? Do you agree that this seems like a bug?

@raamdev raamdev added bug and removed needs testing labels Jul 1, 2014
@jaswrks
Copy link

jaswrks commented Jul 1, 2014

@raamdev From what I can tell, there are two issues going on here.

  1. Quick Cache Pro seems to have lost the value="" attribute in the latest release. That's appears to be a bug in Quick Cache Pro. What a site owner enters into this field is not mirrored back at them in the UI. The option is being saved and implemented, but the UI does not reflect this. See: this line in the source code. Not sure exactly when this happened yet, but we can fix that up in the next release.
  2. A version salt of $_SESSION['user']['PersonalNumber'] on a site that is running in WP_DEBUG mode; will kick back an E_NOTICE from PHP, since this is unique to certain visitors with a session and may not be present at all times. Thus, $_SESSION['user']['PersonalNumber'] is technically invalid, at least some of the time.

I would suggest @$_SESSION['user']['PersonalNumber'] as the version salt. Or, even better:

isset($_SESSION['user']) && !empty($_SESSION['user']['PersonalNumber']) ? $_SESSION['user']['PersonalNumber'] : ''

Another option would be to define the version salt inside /wp-config.php where you have more room to write clean code and build a version salt dynamically. Assign the result of whatever that routine does, to a PHP Constant that would then serve as your version salt.

Example, inside /wp-config.php

if(isset($_SESSION['user']) && !empty($_SESSION['user']['PersonalNumber']))
    define('MY_VERSION_SALT', $_SESSION['user']['PersonalNumber']);
else define('MY_VERSION_SALT', ''); // Default value.

Your Quick Cache version salt would then become...

MY_VERSION_SALT

It's important to keep in mind that Quick Cache (i.e. the advanced-cache.php file in WordPress) loads very early-on. So using a session value may not work at all, it just depends on where/how the session is being started on a given site.

@raamdev
Copy link
Contributor

raamdev commented Jul 1, 2014

Quick Cache Pro seems to have lost the value="" attribute in the latest release. That's appears to be a bug in Quick Cache Pro. What a site owner enters into this field is not mirrored back at them in the UI. The option is being saved and implemented, but the UI does not reflect this. See: this line in the source code. Not sure exactly when this happened yet, but we can fix that up in the next release.

@jaswsinc Thanks. This has been fixed in the dev branch and will go out with the next release.

A version salt of $_SESSION['user']['PersonalNumber'] on a site that is running in WP_DEBUG mode; will kick back an E_NOTICE from PHP, since this is unique to certain visitors with a session and may not be present at all times. Thus, $_SESSION['user']['PersonalNumber'] is technically invalid, at least some of the time.
I would suggest @$_SESSION['user']['PersonalNumber'] as the version salt.

Got it. Would it make sense then to update the inline documentation to include a note at the bottom about that? That PHP notice threw me off, but if I had known that was expected I would've felt better about seeing it.

@jaswrks
Copy link

jaswrks commented Jul 1, 2014

Got it. Would it make sense then to update the inline documentation to include a note at the bottom about that? That PHP notice threw me off, but if I had known that was expected I would've felt better about seeing it

Yes, I agree. It might even be easier if we have a separate KB article to cover this in greater detail. Oh, we actually have one already. Whatever you think is fine with me. Maybe just a warning there... PLEASE READ THIS ARTICLE before using a version salt; and include that in the article.

Thanks. This has been fixed in the dev branch and will go out with the next release.

Great!

@raamdev
Copy link
Contributor

raamdev commented Jul 1, 2014

It might even be easier if we have a separate KB article to cover this in greater detail. Oh, we actually have one already.

Ah, yes, I had forgotten about that! Thanks. I've updated that article with a new section, Why am I seeing PHP Notices related to my version salt?

@raamdev
Copy link
Contributor

raamdev commented Jul 1, 2014

@soukupl The bug you reported has been fixed in the dev version and will go out with the next release.

If you're interested in testing a beta release of Quick Cache before the next version comes out, please sign-up to be a beta tester here.

Please see also @jaswsinc's suggestions above. I'm going to close this issue now unless you have anything else to report.

@raamdev raamdev closed this as completed Jul 1, 2014
@raamdev raamdev added this to the Next Release milestone Jul 1, 2014
@kalligator
Copy link

Could you provide a version of https://github.com/WebSharks/Quick-Cache/blob/000000-dev/quick-cache/includes/ac-plugin.example.php that is ready to be enabled as is? I'm thinking I might be doing something wrong.
I uploaded it in the proper folder, I added some necessary meta in order for it to show under wp plugins, enabled it, but it does not seem to work i.e. it does not vary the cached versions when visiting with a desktop vs an android client.
Did I need to edit anything else in the file to get that functionality working?

Also you may want to add a check before initiating, as it shows Fatal error: Call to a member function add_filter() on a non-object in /home/xxx/public_html/wp-content/plugins/ac-plugins/my-ac-plugin.php on line 49 when quick cache is disabled.

@raamdev
Copy link
Contributor

raamdev commented Aug 26, 2014

@kalligator Please see the documentation in the file that you referenced:

/*
 * If implemented; this file should go in this special directory.
 *    `/wp-content/ac-plugins/my-ac-plugin.php`
 */

You're placing the file in the wrong directory.

Also, in the future, please open a support request for questions like these.

@wpsharks wpsharks locked and limited conversation to collaborators Aug 26, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants