laravel / telescope

An elegant debug assistant for the Laravel framework.
https://laravel.com/docs/telescope
MIT License
4.84k stars 577 forks source link

After upgrading to 5.1.0: Strings with null bytes cannot be escaped. Use the binary escape option. #1489

Closed ahoiroman closed 3 months ago

ahoiroman commented 3 months ago

Telescope Version

5.1.0

Laravel Version

11

PHP Version

8.3

Database Driver & Version

MariaDB

Description

After upgrading laravel/telescope to 5.1.0, I am getting a RuntimeException:

Strings with null bytes cannot be escaped. Use the binary escape option.

Bildschirmfoto 20 06 2024 - 13h 21min 06s@2x

The code that causes this is the following:

$uuid = fake()->uuid();

return Cache::remember("user-$uuid", now()->addHour(), function () {
  return User::create([
    "email" => fake()->email(),
    "password" => bcrypt(fake()->password()),
    "username" => fake()->username()
  ]);
});

The change that introduced this behavior is this: https://github.com/laravel/telescope/pull/1486/files#diff-397191004b8c1dbda4835ec7eda183402511fcb376c12796072958be5ead9f0c

Pinning telescope to 5.0.5 solves the issue for now.

Steps To Reproduce

Create a new Laravel project and use tinker(well) to run this code:

$uuid = fake()->uuid();

return Cache::remember("user-$uuid", now()->addHour(), function () {
  return User::create([
    "email" => fake()->email(),
    "password" => bcrypt(fake()->password()),
    "username" => fake()->username()
  ]);
});
Rockylars commented 3 months ago

I encountered similar issues through any small Model update that made Revisionable call Telescope and fail on it as a Datetime somehow ended up in the escape() function. image Had to lock it to <5.1 for now.

driesvints commented 3 months ago

@morloderex could you provide a fix for this? Otherwise we'll have to revert that PR.

morloderex commented 3 months ago

@driesvints Sorry for the late answer! This issue is something i believe should be fixed in the framework by somehow detecting if the parameter binding while looping should use binary escaping or not since that is not done within substituteBindingsIntoRawSql.

@tpetry can you elaborate a bit more on why you didn't choose get and implement the binary flag for parameter bindings?

billriess commented 3 months ago

Just wanted to chime in that I also noticed this same bug. It was causing our develop database to max connections and eventually crash our vapor environment.

tpetry commented 3 months ago

@tpetry can you elaborate a bit more on why you didn't choose get and implement the binary flag for parameter bindings?

@morloderex There's currently no way in eloquent to provide flags or hints for parameter bindings. The correct way would be to add a binary cast to Eloquent. Its not much work as all the building blocks already exist. But I've just not have any project anymore that stores binary data in the database so I hadn't any need yet to implement it.

driesvints commented 3 months ago

I reverted this PR for now to unblock people. @morloderex would love a new PR but with the things people mentioned here in mind.

JohnnyQ352 commented 1 week ago

Guys the tool is not the problem. It is the older 5.x Laravel code that allowed us to create functions and variables without considering any NULL values for variables or columns within our code calls. so if you had say

public function columbus($user, Request $requedst) { // Your code here... }

You are to redo it like this..

public function columbus($user = null, Request $requedst) { // Your code here... }

and so forth and so on...

In the older Laravel versions we can say

$variable = "a value";

but if it is inside an if statement

if (my_condition){ $variable = "a value";
}

it is not defined at all unless the condition is met so the tool will evaluate it as a string with null bytes instead of a binary with null bytes when it tries to evaluate a condition that was not sent to the function. You can't expect the tool to do everything for you.

In my case I noticed that another tool I used called Pulse, was using the old way of code writing and was giving a crapload of the same error until I checked their code. I simply stop using it and voila..

My point, check your code, validate your data and make sure your variables are pre-defined with null if you need it to be that.

A lot of bool vars can be 0 or 1 as well.

Anyway, just my 2¢

Johnny

ahoiroman commented 1 week ago

To be honest: I dont get your point. At least for my usecase I did not have undefined vars.

if you check my code in the initial post, I dont see any undefined vars at all.