Re: [Bug-wget] Wget 1.11.4 and FTP Server from Windows Server 2008 R2

[Ray Satiro]

_______________________________
> From: Tim Ruehsen <address@hidden>
>To: address@hidden; Ray Satiro <address@hidden> 
>Sent: Thursday, October 4, 2012 9:53 AM
>Subject: Re: [Bug-wget] Wget 1.11.4 and FTP Server from Windows Server 2008 R2
> 
>> I couldn't build 6e4c3ab for MinGW without reverting the recent commit
>> 67e6027 "Add support for file names longer than MAX_FILE" from Tim
>> Ruehsen. I have no pathconf() here and it looks like gnulib doesn't offer
>> it for mingw or msvc. I'm not suggesting it should be reverted in the main
>> repo but there would have to be _PC_NAME_MAX and pathconf() substitutes
>> for windows.
>
>Hi Ray,
>
>since I have no Win Env, could you check if something like this works ?
>
>#ifdef WINDOWS
>ret = PATH_MAX;
>#else
>ret = pathconf (*p ? p : ".", name);
>#endif
>
>It is in utils.c/get_max_length().
>
>I read the following link, but it is not quite clear, how to go on.
>http://stackoverflow.com/questions/833291/is-there-an-equivalent-to-winapis-
>max-path-under-linux-unix
>

I think a lot of the confusion with that in windows programming is that there's 
_MAX_PATH, MAX_PATH, PATH_MAX, and you can go past all that anyway by using the 
extended path prefix with the unicode mapped api. And since the ansi api is in 
most cases just a wrapper around the unicode api you can in a lot of cases use 
the extended path prefix in the ansi api as well. 


The limitations in wget probably depend on gnulib because a lot is wrapped in 
that. gnulib calls mscrt but also direct api. MS says their CRT functions can 
handle paths > MAX_PATH but I haven't always found that to be true, although I 
can't recall a specific example so it's true enough I guess. I never use 
PATH_MAX but I do use MAX_PATH when I'm not using the extended path prefix. 
PATH_MAX varies in windows but MAX_PATH should always be 260 and that includes 
the terminating null. Look at this

http://msdn.microsoft.com/en-us/library/aa365247%28VS.85%29.aspx#maxpath


pathconf() from what I remember was not always reliable/portable but that was 
many years ago so maybe it is these days. How big a path you can use depends on 
what your api accepts so really it's a question of what the gnulib functions 
can handle. Since there's no gnulib pathconf for ms I suggest you do what I do 
and stick with MAX_PATH unless the extended path prefix is in use (I don't see 
it...). Also
there are some caveats with extended path for example you can't do / 
(you do that in your code) and expect it to be converted to a backslash
http://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx#namespaces
regarding your code I can't do unit testing because I don't have the time but 
hopefully someone will. I did notice this
------------
size_t max_length;
[...]
/* Check that the length of the file name is acceptable. */
max_length = get_max_length (fnres.base, fnres.tail, _PC_NAME_MAX) - 
CHOMP_BUFFER;
if (max_length > 0 && strlen (temp_fnres.base) > max_length)
{
[...]
  /* Shorten the file name. */
  temp_fnres.base[max_length] = '\0';
------------

you are assigning to an unsigned type so what if you end up with something like
max_length = 5 - 25;
or slightly more likely
max_length = 0 - 25;

temp_fnres.base[max_length] = '\0';

here you're modifying the string object's buffer directly but not updating its 
tail. if someone else later comes along and adds below that there could be some 
havoc. also when using that object it should be one or the other null 
terminated or not. so lets say you have 

{ 'a', 'b', 'c' } and your tail is 3. you call append_char( '\0', &dest ) then 
you have { 'a', 'b', 'c', '\0' } and your tail is 4 but now what. you need to 
append another string and you do append_string( "def", &dest ); well that's 
going in starting at &base[tail] so you'll have { 'a', 'b', 'c', '\0', 'd', 
'e', 'f' } and your tail is 7. so i'd make it so you have base[tail] as null 
but really i think the usage in the comments to use append_char with a null 
character isn't good or if you must maybe it should be something like
append_char( c, *dest ) { if( !c ) { append_null( dest ); return; } [...] }
append_null( *dest ) { if( dest->size == dest->tail ) { GROW( dest, 1 ); } 
dest->base[tail] = 0; }
but with this lets say you go back and append_string then you're no longer null 
terminated. and further the string object may be intended to hold separated 
strings and i've totally misinterpreted this..