Re: wget2 | Fix accidentally removing fragments when converting links. (!520)

[@rockdaboot]

Tim Rühsen started a new discussion on src/wget.c: 
https://gitlab.com/gnuwget/wget2/-/merge_requests/520#note_1292359016

> -                             }
>  
>                               const char *filename = 
> blacklist_entry->local_filename;
>  
>                               if (config.convert_links) {
>                                       convert_link_whole(filename, 
> conversion, url, &buf);
> -                                     if (blacklist_entry->iri->fragment) {
> +                                     if (iri->fragment) {
>                                               wget_buffer_memcat(&buf, "#", 
> 1);
> -                                             wget_buffer_strcat(&buf, 
> blacklist_entry->iri->fragment);
> +                                             wget_buffer_strcat(&buf, 
> iri->fragment);
>                                       }
>                               } else if (config.convert_file_only)
>                                       convert_link_file_only(filename, url, 
> &buf);
>  
> +                             if (!blacklist_entry)

Good finding and nice a fix !

Here, `blacklist_entry` is always set. If you open the failed runner 
"Sanitizers/Debian" and scroll down, you can see that e.g. `Test-E-k` fails.
You can see the test logs by clicking on "Job Artifacts/Browse" on the right 
side, click on `tests` and then on `Test-E-k.log`.

It says
```
==104292==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 174 byte(s) in 1 object(s) allocated from:
    #0 0x56489217835e in malloc 
(/builds/kuukunen/wget2/src/wget2_noinstall+0xe935e) (BuildId: 
6a05e0440490ee1b7a68c4d80495564bdeabe614)
    #1 0x5648921f31e8 in my_malloc /builds/kuukunen/wget2/src/options.c:4243:12
    #2 0x7f91a581bec2 in wget_malloc 
/builds/kuukunen/wget2/libwget/../include/wget/wget.h:456:9
    #3 0x7f91a581bec2 in wget_iri_parse 
/builds/kuukunen/wget2/libwget/iri.c:493:8
    #4 0x5648921d20b5 in convert_links /builds/kuukunen/wget2/src/wget.c:1148:21
    #5 0x5648921c288e in main /builds/kuukunen/wget2/src/wget.c:1470:3
    #6 0x7f91a5049189 in __libc_start_call_main 
csu/../sysdeps/nptl/libc_start_call_main.h:58:16

SUMMARY: AddressSanitizer: 174 byte(s) leaked in 1 allocation(s).
```

I think the best way to fix it is to introduce a bool variable `free_iri` (or 
whatever name you like) in the block scope with a default of false.
Set it to true where in the original code is `wget_iri_free(&iri)`, and test 
this variable instead of `!blacklist_entry`.

The only runners that should fail in the pipeline are currently `WolfSSL` and 
`pages`. (Sorry for that - needs to be fixed, but my time is very limited atm).

Anyway, I highly appreciate that you took action to fix this issue 
:slight_smile:

-- 
Reply to this email directly or view it on GitLab: 
https://gitlab.com/gnuwget/wget2/-/merge_requests/520#note_1292359016
You're receiving this email because of your account on gitlab.com.