Bug202505190803 undefined variable fatal error

2025-05-19 09:10:59

adminsforum@hubzilla.org

od wrote the following post Mon, 19 May 2025 09:11:20 +0200

Bug202505190803 undefined variable fatal error

It is personal opinion that functionality added without documentation nor quality assurance test, is a bug. Please ignore this post if disagreed.

The bugs as defined by this person alone will only be published within hubzilla. Again, please ignore this post and the author if disagreed.

Bugs are published as information only. Noone is forced or expected to read, reproduce, replicate. Again, please ignore these posts and the author if disagreed.

Attempt to post a message causes fatal error, due to non-fatal undefined variables in the software.

"
Warning: Undefined array key "channel_epubkey"
...
Warning: Undefined array key "channel_eprvkey"
...
Fatal error: Uncaught SodiumException: sodium_crypto_sign_detached(): Argument #2 ($secret_key) must be SODIUM_CRYPTO_SIGN_SECRETKEYBYTES
...
"

show all 14 comments

2025-05-19 11:31:39

Der Pepe (Hubzilla) ⁂ ⚝

pepecyb@hub.hubzilla.hu

@od Please be so kind as to provide more detailed information about where and how the errors occurred. Which hub? Which Hubzilla version? From which log do the messages originate?

In my experience over the years, you will receive excellent help here if you provide sufficient information. Often (but not always), it was just a minor issue, such as a missing library or an outdated version of another package. Or an older add-on was causing problems.

2025-05-19 12:41:03

od@zotum.net

@Der Pepe (Hubzilla) ⁂ ⚝
Consider related to the previous bug observation, from which it should be concluded that more strategic actions should be discussed with those able to take the actions.
How many undefined variables are in the code?
Why are there so many such variables?
How to minimise repetition in future?

2025-05-19 14:04:32

Der Pepe (Hubzilla) ⁂ ⚝

pepecyb@hub.hubzilla.hu

@od Warnings are not such a big problem. Usually, the causes have no effect on functionality. Of course, it would be nice to eliminate them. But that requires resources... someone who knows and understands PHP, who knows and understands the structure of Hubzilla as a PHP application, and who knows how to use git.

Errors and fatal errors, on the other hand, are problematic.

However, such errors are quickly eliminated by the developers as soon as they become known. But they need to be made aware of them. It is quite possible that they do not occur in the developer's context. That is why they rely on such errors being reported... including detailed information about the context (software version, logs, etc.)... THIS is extremely important!

2025-05-19 15:46:11

Harald Eilertsen

harald@hub.volse.no

@Der Pepe (Hubzilla) ⁂ ⚝

Warnings are not such a big problem. Usually, the causes have no effect on functionality.

That's not entirely accurate. A warning simply means that the system (PHP in this case) is able to guess the intention of the code, and continue execution. This guess might not be what the author of the code intended, and may most definitely point to a bug.

Why do these types of errors keep popping up? PHP is getting stricter, and new versions warn about more situations that it previously silently accepted. Also, the language changes over time, so that constructs that before was well defined, may not be so anymore.

Are we doing somthing to address these issues? Yes, but progress is slow.

We are adding automated tests that exercise the various bits of code in a controlled setting. This will uncover problems like these. Both warnings and fatal errors. Currently about 7% of the code base is covered by tests, so a lot of work remains for this to be as useful as it should be.
We are adding static analysis tools that are able to catch mistakes like these, and other problems without actually executing the code. Currently the output from these tools are too noisy to be really useful, but as the code improves so will the value of these tools.
There's also ongoing work to make use of more modern PHP features that will improve the safety and correctness of the code.

All of this is as expected with a codebase that has been under development without any tests or static analysis tooling for as long as Hubzilla. This is a complex project that has a lot of moving parts, and has been in development through several versions of the PHP language. Many language features and techniques that are available today wasn't available when the project started. Moving things toward using safer constructs without breaking existing functionality is a long process that requires a lot of care and studying the code. Better test and tool coverage will also help this process.

2025-05-19 15:53:49

Harald Eilertsen

harald@hub.volse.no

Forgot to mention: sodium_crypto_sign_detached is called exactly once in the code, so it's not too hard to find. The problem is that the code in Zotlabs\Lib\JcsEddsa2022::sign does not validate that the $channel passed in is an array, and that it contains the element keys required for the function.

The warnings are in this case most definitely due to a bug.

2025-05-19 15:59:00

od@zotum.net

@Harald Eilertsen
A value of 7 % is a surprise; informative

2025-05-19 16:05:00

Harald Eilertsen

harald@hub.volse.no

@od It's up from 0% when I started :) But yeah, lots of work remains.

2025-05-19 16:09:07

od@zotum.net

@Harald Eilertsen
Wow! There should be a "freeze" on any new bugs (sorry, "features") until this quality figure is much, much higher

2025-05-19 16:33:00

Der Pepe (Hubzilla) ⁂ ⚝

pepecyb@hub.hubzilla.hu

@od

There should be a "freeze" on any new bugs (sorry, "features") until this quality figure is much, much higher

You don't really mean that the dev code should remain frozen until the amount of automatically tested code is as large as you think it should be... and until there are no more warnings and errors? What are YOU going to do to help achieve this in a very short time?

2025-05-19 16:56:06

od@zotum.net

@Der Pepe (Hubzilla) ⁂ ⚝
Nobody has to do anything, but seems from the information shared that a lot of quality tests are necessary. It is not understood why the work has to be done in a very short time. Better to make good, slow improvements.
Instead of "move fast, break things" (according to that certain social media multi-millionaire!), "move slow, fix things" ;)

2025-05-19 17:02:31

Der Pepe (Hubzilla) ⁂ ⚝

pepecyb@hub.hubzilla.hu

@od I haven't noticed anything yet that has been ‘destroyed’ by the further development. And I haven't noticed any drastic effects from the remaining bugs either.

But feel free to help us fix any bugs you find.

2025-05-19 17:11:31

od@zotum.net

@Der Pepe (Hubzilla) ⁂ ⚝

o help us fix any bugs

To increase participation:
authors should feel confident that their content is going to be valid for a mutually agreed time;
programmers should believe that their code contribution will be robust, interconnected well and not likely to impact the contributions of others;
passive participants ("users") are respected for giving their observations when they use the software.

If people are asked to fill a tank with a bucket and the bucket has an increasing size hole, it is reasonable to expect people to lose motivation.

2025-05-19 18:09:07

Der Pepe (Hubzilla) ⁂ ⚝

pepecyb@hub.hubzilla.hu

@od The programmers at Hubzilla set their own pace. That's not the problem.

What is important (!) is that the error messages from users are also usable. It doesn't help to post a few excerpts from the logs where something with ‘Warning’ can be found. The context must always be provided. Specifically:

1. It is best to provide the complete log with the warning or error message.
2. Which version of the software was used?
3. During which function/activity did the error occur?
4. When did the error occur (time)?
5. How did the error affect the user?

So... there we have the problem! Pure users cannot provide this information. They don't even know that a warning or error appears in the log because they don't see the logs. Only the hub administrator can see them. They can only report when something is not working or when something is not working as expected or is completely wrong. In this case, it is best to contact the administrator of your hub, tell them what you have noticed and, if possible, give them the approximate time. The admin can then check the log to see if they can find anything. They may be able to fix the error themselves (incorrect configuration or error in the server system). If not, they will contact the developers either via the admin forum or the bug tracker of the repository.

The user can therefore only report any faulty behaviour they notice and provide information about the version running on their hub. They may also provide the time at which the error occurred (and whether it occurred after a hub update). That is all they can do. The developers are then left in the dark unless the cause of the error is obvious to them.

Now YOU have the advantage of being both a user and an admin (because that's the only way you can see the error messages from the logs). It would now be important to post the complete log or a generous excerpt from the log around the time the error occurred. AND, if necessary, how the error affected the user. Then work can begin.

2025-05-20 10:52:00

Harald Eilertsen

harald@hub.volse.no

Dug some more into this:

JcsEddsa2022::sign is only ever called from Activity::build_packet, which by default sets an empty array for the $channel arg if not specifies by the caller. However, that is fine. It will not try to sign the packet if a $channel has not been provided.

The most likely problem caller of Activity::build_packet is the as_return_and_die function, which is called from a number of different places, also addons.

It would be useful to have a backtrace of when this occurred, so that I could see which code path lead to the error.

I've submitted a draft MR to address the issue for now: https://framagit.org/hubzilla/core/-/merge_requests/2201/diffs

It does not solve the issue, but adds some validation to the JcsEddsa2022::sign method, and throws an exception if the required conditions are not met. This will get rid of the warnings, and provide a bit more meaningful information when the crash happens.

I'll try to look more into it later. Now I have to work.