[connman] Apply upstream fix for HTTP response crash. Fixes JB#59564 #35
Conversation
If an http response has a line starting with whitespace before the first valid header field, g_hash_table_lookup() is called with a NULL key, which is invalid. Fix this by checking for a valid last_key before attempting to process continuation lines. Based on a patch by Tomasz Bursztyka for an issue originally reported by Marcel Mulder.
LaakkonenJussi
force-pushed
the
jb59564
branch
from
c5bf334
to
f71a5af
Compare
LaakkonenJussi
force-pushed
the
jb59564
branch
from
f71a5af
to
c1584ef
Compare
GError was not cleared after status URL checks are done when checking
the configuration file. This caused an obvious error when the
configuration file did not have the status URLs defined:
(connmand:1070): GLib-WARNING **: 12:19:38.963: GError set over the top
of a previous GError or uninitialized memory. This indicates a bug in
someone's code. You must ensure an error is NULL before it's set. The
overwriting error message was: Key file does not have key ?Enable6to4?
in group ?General?
As the error was already ignored for IPv6 we can drop it also for IPv4
case hence the default is returned in connman_setting_get_string() when
the ipv{4,6}_status_url is not set (NULL).
LaakkonenJussi
force-pushed
the
jb59564
branch
from
c1584ef
to
8af3440
Compare
There was a problem hiding this comment.
I would just suggest to 1) add the common "(cherry picked from commit ...)" note to the first commit and 2) consider to tidy up the changelog messages (the one in PR covers both commits while the second commit comes with its own message too. So maybe use own message for each commit?)
Thanks. I don't usually edit the upstream commit texts, and not the commits themselves if they apply without conflicts. The logic here is to keep the tracing of the commits easier, I usually put commits that can be upstreamed with both formats:
So from history it is easy to detect that if the change is fork only then it has []-tag and the change in case of upgrade (hopefully someday...) needs to be kept and changing that part might be bad for us. I've been having these summary texts in the PR titles as that is aiming to give a good overall view on what was done in that particular PR. Then the commit messages can detail the specific changes. |
The easiest to trace are the commits with "(cherry picked from commit ...)" note as the
Yeah, summary in PR message is great, but keep in mind that if you write it as a changelog item, then it will appear in changelog on the same level as the items from commits. Having one item that repeats what other items on the same level say feels odd. |
This approach has been never used with this fork. At this point it might not be any additional benefit until we update to some baseline release from upstream.
I'm well aware of this. It really does not bother me. |
LaakkonenJussi
changed the title
Applies an upstream patch: "gweb: Fix crash on malformed http response"
22901212105055b24f73504a74f5a57eee809777
to address issue caused by a CVE-fix "gweb: Fix OOB write in received_data()"
d1a5ede5d255bde8ef707f8441b997563b9312bd.
Also drop the use of errors for reading IPv4 status url from the configuration file. With
IPv6 this was also ignored and the default is being returned when the status url is not
set. This caused a Glib warning as GError was not cleared.