Skip to content

Conversation

@koreus
Copy link
Contributor

@koreus koreus commented Feb 2, 2025

Context : My admin preferences (modules & main) was not always saved.

htdocs/modules/system/themes/ComposerInfo.php

I checked and found an error 404 in the browser network tab. It was from the file : htdocs/modules/system/themes/ComposerInfo.php . I comment the line but it didn't resolve the problem. But I think it's better commented.

htdocs/class/xoopssecurity.php

I found that an invalid token was aborting the save with a redirection.
The token was not saved in the session (xoops_session table) but was set correctly in $_SESSION
So I forced the updating of the session in database with add session_write_close();

htdocs/kernel/session.php

I had an SQL duplicate index error with an INSERT comment in xoops_session
The fault was this test if (!$this->db->getAffectedRows()) after and UPDATE in the public function write($sessionId, $data)

If a row exist and is not updated (all same values), getAffectedRows return 0 and the code will try to INSERT, which cause an error (duplicate index)

Copy link
Collaborator

@mambax7 mambax7 left a comment

Choose a reason for hiding this comment

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

Seems to be OK. Thank you!
How about adding the change to prevent direct access?

@mambax7
Copy link
Collaborator

mambax7 commented Feb 4, 2025

Can you add some tests, so we can make sure that it works and doesn't break something?

@koreus
Copy link
Contributor Author

koreus commented Feb 4, 2025

Can you add some tests, so we can make sure that it works and doesn't break something?

How I do that ? Do you have an example ?

@mambax7 mambax7 merged commit e939add into XOOPS:master Feb 5, 2025
4 checks passed
@mambax7
Copy link
Collaborator

mambax7 commented Mar 15, 2025

@koreus can you please check if the #1525 is still addressing your original problem?

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants