[vlc-devel] [PATCH] Accept port specifications in smb2, now that the underlying library supports them.

[Thomas Guillem]

Merged, thanks ! And sorry for the long delay.

On Mon, Jun 3, 2019, at 17:49, Hugo Beauzée-Luyssen wrote:
> On Mon, Jun 3, 2019, at 5:23 PM, Michael Young wrote:
> > On 6/3/19 9:04 AM, Michael Young wrote:
> > > On 6/3/19 8:17 AM, Hugo Beauzée-Luyssen wrote:
> > >> On Mon, Jun 3, 2019, at 1:31 PM, Michael Young wrote:
> > >>> ---
> > >>>   modules/access/smb2.c | 10 ++--------
> > >>>   1 file changed, 2 insertions(+), 8 deletions(-)
> > >>>
> > >>> diff --git a/modules/access/smb2.c b/modules/access/smb2.c
> > >>> index 41864ccfa8..7374956dea 100644
> > >>> --- a/modules/access/smb2.c
> > >>> +++ b/modules/access/smb2.c
> > >>> @@ -58,8 +58,6 @@
> > >>>   #include "smb_common.h"
> > >>> -#define CIFS_PORT 445
> > >>> -
> > >>>   static int Open(vlc_object_t *);
> > >>>   static void Close(vlc_object_t *);
> > >>> @@ -583,10 +581,6 @@ Open(vlc_object_t *p_obj)
> > >>>       if (vlc_UrlParseFixup(&sys->encoded_url, access->psz_url) != 0)
> > >>>           return VLC_ENOMEM;
> > >>> -    if (sys->encoded_url.i_port != 0 && sys->encoded_url.i_port != CIFS_PORT)
> > >>> -        goto error;
> > >>> -    sys->encoded_url.i_port = 0;
> > >>> -
> > >>>       sys->smb2 = smb2_init_context();
> > >>>       if (sys->smb2 == NULL)
> > >>>       {
> > >>> @@ -598,10 +592,10 @@ Open(vlc_object_t *p_obj)
> > >>>           sys->encoded_url.psz_path = (char *) "/";
> > >>>       char *resolved_host = vlc_smb2_resolve(access, sys->encoded_url.psz_host,
> > >>> -                                           CIFS_PORT);
> > >>> +                                           mwy-cpjbvr at the-youngs.org);
> > >>>       /* smb2_* functions need a decoded url. Re compose the url from the
> > >>> -     * modified sys->encoded_url (without port and with the resolved host). */
> > >>> +     * modified sys->encoded_url (with the resolved host). */
> > >>>       char *url;
> > >>>       if (resolved_host != NULL)
> > >>>       {
> > >>
> > >> Hi,
> > >>
> > >> AFAICS there is no libsmb2 release including this feature, what happens when a port is provided to an old version of the library?
> > >>
> > > 
> > > The host:port would be interpreted as a host name, and the open would fail.
> > > 
> > > It would be reasonable to transform the default port to an empty port specification with:
> > >    if (sys->encoded_url.i_port =)= CIFS_PORT) sys->encoded_url.i_port = 0;
> > > in case it is provided explicitly.  I don't see anything adding the port number if it's
> > > not provided, but I may have missed something.
> > > 
> > > Where does videolan get the version of libsmb2 for its builds?
> > > I can pick that up and verify the behavior.
> > 
> > I rebuilt with release v2.0.0 from sahlberg/libsmb2, and can confirm 
> > that the open fails when
> > a port number is provided.  An excerpt of a log with an explicit port 
> > number:
> > 
> > [000055817151a640] main input debug: 
> > `smb://127.0.0.1:445/music/flac/Buddy Guy/Sweet Tea/01_Done Got 
> > Old.flac' gives access `smb' demux `any' path 
> > `127.0.0.1:445/music/flac/Buddy Guy/Sweet Tea/01_Done Got Old.flac'
> > [00007fb848000df0] main access debug: creating access: 
> > smb://127.0.0.1:445/music/flac/Buddy Guy/Sweet Tea/01_Done Got Old.flac
> > [00007fb850b0d8e8] main generic debug: looking for access module 
> > matching "smb": 224 candidates
> > [00007fb848000df0] smb2 access error: smb2_connect_share_async failed: 
> > Invalid address:127.0.0.1:445  Can not resolv into IPv4/v6.
> > [00007fb848000df0] main access error: SMB2 operation failed
> > [00007fb848000df0] main access error: Invalid address:127.0.0.1:445  
> > Can not resolv into IPv4/v6.
> > ... repeated several times
> > [00007fb850b0d8e8] main generic debug: no access modules matched
> > [000055817151a640] main input error: Your input can't be opened
> > [000055817151a640] main input error: VLC is unable to open the MRL 
> > 'smb://127.0.0.1:445/music/flac/Buddy Guy/Sweet Tea/01_Done Got 
> > Old.flac'. Check the log for details.
> > [000055817151a640] main input debug: destroying input for '01_Done Got 
> > Old.flac'
> > 
> > With no port number (or explicitly setting 0), the open succeeds:
> > [000055feecd59580] main input debug: 
> > `smb://127.0.0.1:0/music/flac/Buddy Guy/Sweet Tea/01_Done Got Old.flac' 
> > successfully opened
> > 
> > I've inquired about cutting a new release of libsmb2 to pick up recent changes.
> > 
> 
> Thank you! Feel free to submit another patch when the version gets released :)
> 
> > In the meanwhile, I think the result of using a port number against 
> > libsmb2 v2.0.0 is no worse
> > than ignoring one, with the possible exception of explicitly supplying 
> > ":445", which I'd be
> > happy to translate to 0 as a precaution.
> > 
> > Let me know what you think.
> > 
> 
> Indeed since we were already silently ignoring the port number, that's 
> probably fine :)
> 
> Thanks!
> 
> -- 
>   Hugo Beauzée-Luyssen
>   hugo at beauzee.fr
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel