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

error from mysql.inc.php #848

Closed
bauigel opened this issue Jul 17, 2024 · 4 comments · Fixed by #870
Closed

error from mysql.inc.php #848

bauigel opened this issue Jul 17, 2024 · 4 comments · Fixed by #870

Comments

@bauigel
Copy link

bauigel commented Jul 17, 2024

With current git-master I get an error from mysql.

Notice: Only variable references should be returned by reference in /include/db/mysqli.inc.php on line 101.
@bauigel
Copy link
Author

bauigel commented Jan 19, 2025

ChatGPT offers two solutions for this

function serendipity_db_escape_string($string) {
    global $serendipity;

    if ($string === null) {
        return '';
    }

    return mysqli_real_escape_string($serendipity['dbConn'], $string);
}

or using mysqli_real_escape_string instead of mysql_escape_string

function serendipity_db_escape_string($string) {
    global $serendipity;

    return mysqli_real_escape_string($serendipity['dbConn'], $string ?? '');
}

For my test I used the second solution and it works for me.

@onli
Copy link
Member

onli commented Feb 1, 2025

@bauigel The error you quote does not fit to the proposed fixes. The error message speaks about a reference, but the two patches add null treatment handling to serendipity_db_escape_string, which is not on line 101 and not called there. I'm unsure how to continue :/

Ignoring the error message mismatch: It might be useful to let serendipity_db_escape_string return an empty string if it is given a null, but it also might hide bugs. Just yesterday I ran into that, when writing the test cases for #865 at one point the null warning helped.

Could you have a second look at the error message - is it really gone, and really with this change?

@bauigel
Copy link
Author

bauigel commented Feb 9, 2025

Ah, sorry my fault. The "solution" is for another problem I had.

The error on line 101 should be fixed by removing the & from line 61:

function serendipity_db_query(...

onli added a commit that referenced this issue Feb 10, 2025
Also adjusts the code that calls the function with a prepended &, as that would have caused a new warning. Fixes #848
@onli
Copy link
Member

onli commented Feb 10, 2025

Interestingly, I did not get the quoted error message, but a similar one when removing the & - in the functions that called the serendipity_db_query as $somevar =& serendipity_db_query(...) . I have to admit I'm not sure about the context here, but I opened a PR with a fix that seemed to work for me. We'll see.

@onli onli closed this as completed in #870 Mar 5, 2025
onli added a commit that referenced this issue Mar 5, 2025
* Remove & reference declaration from db_query to avoid warning
Also adjusts the code that calls the function with a prepended &, as that would have caused a new warning. Fixes #848

* Fix reference return warning for getFile
Is also not returning a sharable variable

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

Successfully merging a pull request may close this issue.

2 participants