Re: [Bug-wget] Misuse of idn2_free()
On Samstag, 8. April 2017 18:55:12 CEST Gisle Vanem wrote: > > Ups, I was sure that we already reallocate the idn2 memory in iri.c/ > > idn_encode(), but we don't do that yet. > > You're confusing Wget with Wget2 :-) Yes, I guess so too :-) > > Could you please try this patch (on top of the current master): > > > > diff --git a/src/iri.c b/src/iri.c > > index 2a4da1de..75eaeea6 100644 > > Work fine! Thanks for feedback ! Regards, Tim signature.asc Description: This is a digitally signed message part.
Re: [Bug-wget] Misuse of idn2_free()
A bit simpler patch:
--- a/url.c 2017-04-08 11:24:21
+++ b/url.c 2017-04-08 12:01:07
@@ -943,7 +943,8 @@
if (new)
{
xfree (u->host);
- u->host = new;
+ u->host = xstrdup (new);
+ idn2_free (new);
host_modified = true;
}
}
--
--gv
Re: [Bug-wget] Misuse of idn2_free()
Ups, I was sure that we already reallocate the idn2 memory in iri.c/
idn_encode(), but we don't do that yet.
Could you please try this patch (on top of the current master):
diff --git a/src/iri.c b/src/iri.c
index 2a4da1de..75eaeea6 100644
--- a/src/iri.c
+++ b/src/iri.c
@@ -290,6 +290,13 @@ idn_encode (const struct iri *i, const char *host)
xfree (utf8_encoded);
+ if (ret == IDN2_OK && ascii_encoded)
+{
+ char *tmp = xstrdup (ascii_encoded);
+ idn2_free (ascii_encoded);
+ ascii_encoded = tmp;
+}
+
return ret == IDN2_OK ? ascii_encoded : NULL;
}
I can also see that idn_decode() can be resurrected with the new libidn2 2.0.0
functionality. I'll do that within the next days.
Regards, Tim
On Samstag, 8. April 2017 11:52:59 CEST Gisle Vanem wrote:
> Tim Rühsen wrote:
> > Thanks, Gisle.
> >
> > pushed with several additional fixes/cleanups regarding idn2.
>
> Too much cleanups I guess since it's crashing because no
> 'idn2_free()' called when needed. This works here:
>
> --- a/url.c 2017-04-08 11:24:21
> +++ b/url.c 2017-04-08 11:38:37
> @@ -944,6 +944,7 @@
> {
> xfree (u->host);
> u->host = new;
> + u->idn_allocated = true;
> host_modified = true;
> }
> }
> @@ -1222,6 +1223,9 @@
> {
> if (url)
> {
> + if (url->idn_allocated)
> +idn2_free (url->host);
> + else
> xfree (url->host);
>
> xfree (url->path);
>
> --- a/url.h 2017-04-08 11:24:21
> +++ b/url.h 2017-04-08 11:35:26
> @@ -84,6 +84,7 @@
> enum url_scheme scheme; /* URL scheme */
>
> char *host; /* Extracted hostname */
> + bool idn_allocated; /* 'host' allocated by libidn2 */
> int port; /* Port number */
>
> ---
signature.asc
Description: This is a digitally signed message part.
Re: [Bug-wget] Misuse of idn2_free()
Tim Rühsen wrote:
Thanks, Gisle.
pushed with several additional fixes/cleanups regarding idn2.
Too much cleanups I guess since it's crashing because no
'idn2_free()' called when needed. This works here:
--- a/url.c 2017-04-08 11:24:21
+++ b/url.c 2017-04-08 11:38:37
@@ -944,6 +944,7 @@
{
xfree (u->host);
u->host = new;
+ u->idn_allocated = true;
host_modified = true;
}
}
@@ -1222,6 +1223,9 @@
{
if (url)
{
+ if (url->idn_allocated)
+idn2_free (url->host);
+ else
xfree (url->host);
xfree (url->path);
--- a/url.h 2017-04-08 11:24:21
+++ b/url.h 2017-04-08 11:35:26
@@ -84,6 +84,7 @@
enum url_scheme scheme; /* URL scheme */
char *host; /* Extracted hostname */
+ bool idn_allocated; /* 'host' allocated by libidn2 */
int port; /* Port number */
---
--
--gv
Re: [Bug-wget] Misuse of idn2_free()
Thanks, Gisle.
pushed with several additional fixes/cleanups regarding idn2.
Regards, Tim
On Samstag, 8. April 2017 10:09:22 CEST Gisle Vanem wrote:
> The 'idn_decode()' function now simply uses 'xstrdup()'.
> And in host.c + connect.c there are calls to 'idn2_free()'
> on this pointer:
> if (opt.enable_iri && (name = idn_decode ((char *) print)) != NULL)
>{
> int len = strlen (print) + strlen (name) + 4;
> str = xmalloc (len);
> snprintf (str, len, "%s (%s)", name, print);
> str[len-1] = '\0';
> idn2_free (name); << !
>}
>
> Since the above 'name' is NOT from libidn, I get a crash when
> mixing a MinGW built libidn2.dll with a MSVC built Wget.exe.
>
> I think someone forgot to change the above code when 'idn_decode()'
> got simplified. This patch works for me:
>
> --- a/connect.c 2017-01-19 21:37:55
> +++ b/connect.c 2017-04-08 09:57:24
> @@ -284,7 +284,7 @@
> str = xmalloc (len);
> snprintf (str, len, "%s (%s)", name, print);
> str[len-1] = '\0';
> - idn2_free (name);
> + xfree (name);
> }
>
> logprintf (LOG_VERBOSE, _("Connecting to %s|%s|:%d... "),
>
> --- a/host.c 2017-01-19 21:37:55
> +++ b/host.c 2017-04-08 10:02:42
> @@ -850,7 +850,7 @@
> str = xmalloc (len);
> snprintf (str, len, "%s (%s)", name, host);
> str[len-1] = '\0';
> - idn2_free (name);
> + xfree (name);
> }
>
> logprintf (LOG_VERBOSE, _("Resolving %s... "),
>
>
>
> --gv
signature.asc
Description: This is a digitally signed message part.
[Bug-wget] Misuse of idn2_free()
The 'idn_decode()' function now simply uses 'xstrdup()'.
And in host.c + connect.c there are calls to 'idn2_free()'
on this pointer:
if (opt.enable_iri && (name = idn_decode ((char *) print)) != NULL)
{
int len = strlen (print) + strlen (name) + 4;
str = xmalloc (len);
snprintf (str, len, "%s (%s)", name, print);
str[len-1] = '\0';
idn2_free (name); << !
}
Since the above 'name' is NOT from libidn, I get a crash when
mixing a MinGW built libidn2.dll with a MSVC built Wget.exe.
I think someone forgot to change the above code when 'idn_decode()'
got simplified. This patch works for me:
--- a/connect.c 2017-01-19 21:37:55
+++ b/connect.c 2017-04-08 09:57:24
@@ -284,7 +284,7 @@
str = xmalloc (len);
snprintf (str, len, "%s (%s)", name, print);
str[len-1] = '\0';
- idn2_free (name);
+ xfree (name);
}
logprintf (LOG_VERBOSE, _("Connecting to %s|%s|:%d... "),
--- a/host.c 2017-01-19 21:37:55
+++ b/host.c 2017-04-08 10:02:42
@@ -850,7 +850,7 @@
str = xmalloc (len);
snprintf (str, len, "%s (%s)", name, host);
str[len-1] = '\0';
- idn2_free (name);
+ xfree (name);
}
logprintf (LOG_VERBOSE, _("Resolving %s... "),
--gv
