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

[REF] Fix error where entryURL does not contain id of the contributio… #19917

Merged

Conversation

seamuslee001
Copy link
Contributor

…n page when coming from a shortcode which leads to infinite redirect loop if session timesout

Overview

This fixes a slightly unusal issue that can create a very bad situation. To reproduce you can use a local buildkit build of wordpress using the wp-demo build.

Navigate to https:///contribution-page/ and fill in the page and then navigate to the Confirm step.

Now the important thing is to let it sit there for 10-15m, however if you are not patient then you can login to MySQL and truncate the civicrm_cache table (i.e. basically destroy the current session information).

Now if you click back your meant to be taken back to the original main step of the process with a status message saying "We have reset your session because it timed out" or similar.

Before

Infinite loop created

After

No Infinite loop and user is correctly taken back to the original page and status message shows up.

Technical Details

The issue is that the entryUrl doesn't contain the page id but more importantly doesn't have reset=1 to trigger the reset of the session when you are coming from a page built using shortcode

ping @eileenmcnaughton @haystack @kcristiano @JoeMurray

@civibot
Copy link

civibot bot commented Mar 26, 2021

(Standard links)

@civibot civibot bot added the master label Mar 26, 2021
@kcristiano
Copy link
Member

@seamuslee001 I'll take a look and do an r-run @christianwach may have further comments. We discussed and I don't think he has a better way to handle.

@christianwach
Copy link
Member

As I said on MM, this seems to fix the issue - although the final URL isn't the default URL for the post where the shortcode is embedded. Without making special checks for WordPress shortcodes here, I don't see a better solution.

@kcristiano
Copy link
Member

@seamuslee001

I had reproduced the error. With the patch I do not get an infinite loop. But if I destroy the cache I get a fatal:

Could not find valid value for id

If I submit without waiting/dumping the cahe the form submits correctly

@seamuslee001
Copy link
Contributor Author

Right well I think this is an improvement then. I'll think about how the entryUrl might be helpful but I would see that as a follow up pr thing @eileenmcnaughton

@eileenmcnaughton
Copy link
Contributor

erm - I don't have my head in this - so if it's agreed that it should be merged by @kcristiano then I will - if you need me to decide I'll have to come back to it

@agileware-justin
Copy link
Contributor

Good one @seamuslee001

We've seen this problem on the Event Registration pages as well.

@mattwire
Copy link
Contributor

mattwire commented Apr 9, 2021

I've seen this kind of thing and agree this will be an improvement. Before merge can we get a comment briefly explaining why we need that line of code so the next time a developer looks at that bit they know what it's for.

@kcristiano
Copy link
Member

@seamuslee001 I've tested again this time on Ubuntu 20.04, php-fpm 7.3 and Apache 2.4 - still WP 5.7 and CiviCRM Master. lat test was on my macbook.

Kill caches - even restart php-fpm. Then submit

image

I think the earlier issue was a local issue.

I agree a code comment should be added, otherwise I think this is good to merge.

@eileenmcnaughton
Copy link
Contributor

@seamuslee001 please go ahead & self-merge this once you have added the requested comment

@seamuslee001 seamuslee001 force-pushed the wordpress_shortcode_timeout branch from 56f0249 to 8ccc94a Compare April 12, 2021 08:42
@seamuslee001
Copy link
Contributor Author

ok I have put a comment message in saying why we are doing this so I think we are right to go now

…n page when coming from a shortcode which leads to infinite redirect loop if session timesout

Add in comment and expand fix to cover event registratons as well
@seamuslee001 seamuslee001 force-pushed the wordpress_shortcode_timeout branch from 8ccc94a to 093fd90 Compare April 12, 2021 08:44
@eileenmcnaughton eileenmcnaughton merged commit de02266 into civicrm:master Apr 12, 2021
@eileenmcnaughton eileenmcnaughton deleted the wordpress_shortcode_timeout branch April 12, 2021 19:49
@agileware-justin
Copy link
Contributor

agileware-justin commented Apr 14, 2021

Screenshot_20210414_120228

@kcristiano
Copy link
Member

@agileware-justin any comments on the above? is this after the patch and forcing a cache-clear? also what cms role was in use?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants