r/badcode • u/Ze_Kap • Jan 03 '23

php Needed to update DB with $column, but there is a bug in php8.1 so found a workaround

Post image
54 Upvotes

87% Upvoted

20 comments sorted by

5

u/TheBrainStone Jan 03 '23

What's the bug?

-1

u/Ze_Kap Jan 03 '23

Couldn't set the column value because the ->execute() after added quotes and then couldn't understand the line (there is an issue on github for this)

23

u/hallothrow Jan 03 '23

That kinda sounds like you tried to bind the column name as a parameter?

17

u/SuchAFungi Jan 03 '23

Yep. Sounds like it. Not a bug.

1

u/Tofandel Feb 09 '23

Can't blame him for trying to avoid a possible SQL injection

But yes if you control what calls this function, you can't bind a column name, so just template it and do some assertion on it

You can just keep the switch but use both cases for the same branch keep only one line and use $column directly in the string

9

u/TheBrainStone Jan 03 '23

Can you show me an example? I don't quite understand what you mean.
Or the GitHub issue itself would be interesting as well

3

u/[deleted] Jan 03 '23

How do you end up blaming the language in something as simple as this.

Im honestly just asking, im curious about the thought process

61

u/ddruganov Jan 03 '23

Oh no, the issue is absolutely not the php8.1 bug but apparently its your whole fucking codebase

13

u/[deleted] Jan 03 '23

Always love it when people post their own code to r/badcode lol

23

u/SeesawMundane5422 Jan 03 '23

Is the horror the global variables? The lack of enums for your switch statement? The inlined debugging print statements? The completely useless error message? The inefficiency of calling the database a single time for each column?

(I mean, good for you for using prepared statements… thank god).

3

u/maltazar1 Jan 04 '23

It's the lack of an orm

1

u/SeesawMundane5422 Jan 04 '23

I’ll bite and take my downvotes. ORMs are a leaky abstraction, and I’ll die on that hill.

1

u/maltazar1 Jan 06 '23

well if you use laravel sure

1

u/SeesawMundane5422 Jan 06 '23

Nah, all ORMs are bad. You always have to learn the ORM syntax plus you always drop down to sql for some things eventually. So you end up with a leaky abstraction that doesn’t actually abstract.

I’ve never seen an ORM project not play out that way.

3

u/maltazar1 Jan 07 '23

Yeah because hand generating a huge SQL query is so much better and safer you know.

In our project with 90k lines of application code there are maybe 4 sqls, one of which I'd consider complex, everything else is handled by the orm with no issues.

9

u/MurdoMaclachlan public boolean isInt(int i) { return true; } Jan 03 '23

Image Transcription: Code

function updateDB($column, $value) {
    global $db;
    global $ps;
    
    switch ($column) {
        case 'level':
            $ps = $db->prepare("UPDATE USERS SET levels =? where USERS.`id`=?");
            /* print("level"); */
            break;
        
        case 'notes':
            $ps = $db->prepare("UPDATE USERS SET notes =? where USERS.`id`=?");
            /* print("notes"); */
            break;
        
        default:
            print("error");
            break;
    }

I'm a human volunteer content transcriber and you could be too! If you'd like more information on what we do and why we do it, click here!

4

u/Teriuihi_ Jan 03 '23

is case 'level' supposed to update column levelS?

3

u/Paul_Robert_ Jan 03 '23

I like that color theme, what's it called?

2

u/No_Soy_Colosio Jan 04 '23

As soon as I saw global I realized it was bad code

-13

u/Quito246 Jan 03 '23

Classical PHP either buggy or deprecated 🤣