cURL / Mailing Lists / curl-library / Single Mail

curl-library

Re: PATCH: ftp.c force directory exists on upload, minor bugfix.

From: Early Ehlinger <early_at_respower.com>
Date: Tue, 5 Aug 2003 11:52:05 -0500

"Daniel Stenberg" <daniel_at_haxx.se> wrote:
> Thanks Early for your contribution!
>
> Please use 'diff -u' and resend. Otherwise I'll get problems applying it
in my
> end!

diff -u curl-7.10.6/lib/ftp.c orig/curl-7.10.6/lib/ftp.c > ftp.diff

Attached - I don't believe I've changed it any since yesterday.

> > When uploading with libcurl, I found that if you reuse the connection,
> > ftp->dirs does not get reset correctly and as a result files often end
up in
> > the wrong spot on the remote server.
>
> It is truly interesting that you post this only minutes after Salvatore
> Sorrentino posted bug report #783116:
>
http://sourceforge.net/tracker/index.php?func=detail&aid=783116&group_id=976
&atid=100976
>
> I would still like to see a command line that shows this failure
happening. It
> would just rock to get a test case added that does it.
>
> Then I would like a fix that only cures the problem without adding any new
> features, please please post the new features patch separately!

Being a curl newby, I'm not sure that curl.exe can exhibit this problem; I'm
not even sure if it handles reusing the connection for uploading multiple
files. I'll see if I can find some time to boil down the problem to a
simple libcurl test program, but I'm on a really tight schedule... In my
case, the bug went something like this:

Upload to the following URLs (reusing the connection):

ftp://someserver/file1.txt
ftp://someserver/directory1/file2.txt
ftp://someserver/file3.txt

You would see on the server:

file1.txt
directory1/file2.txt
directory1/file3.txt

> > NEW FEATURE: I added two functions, ftp_mkd and ftp_force_cwd. ftp_mkd
does
> > what you would expect - it creates a directory. ftp_force_cwd does a
CWD,
> > and does a MKD if necessary.
>
> > I updated the code that calls ftp_cwd before an upload to call
ftp_force_cwd
> > instead. This really should be an option instead, but as I'm still a
> > libcurl newbie I do not feel comfortable enough with the sources to make
> > this change. If somebody would like to point me in the right direction,
I
> > should have the time in the next few days to make this patch a little
more
> > robust.
>
> I do not think this is a good default behaviour. We could discuss wheather
> this is even necessary as an option, as you can easily add a custom MKD
> command as a QUOTE one. Care to explain why you feel this feature should
be
> included?

I agree, this is definitely *NOT* what you would usually want libcurl to do
by default. In fact, with the patch the way it is, it *might* cause
directories to be created during a download, unless I'm missing something.
Clearly BAD. This is why I'd rather it be a curl_easy_setopt option,
something like CURL_FTP_FORCE_REMOTE_DIRECTORY.

The reason I think it should be included is that there is no obvious way for
an application to determine the presence of a directory in order to decide
if the directory should be created in the first place. In other words,
there doesn't seem to be an option for "does this directory exist? don't
download its index or anything, just tell me if it's there." That's kind of
a prerequisite to deciding whether you should send the MKD. The only way I
can see to make that determination in the confines of FTP is to attempt to
CWD to the directory, and if that fails, do an MKD. (Which is what
ftp_force_cwd does).

On a more practical level, if I'm trying to upload a file in some
subdirectory that might be many levels deep, then it's a real burden for the
application to parse the path, do all the mkd's, etc., especially when
libcurl already does *everything* involved except for the mkd-on-failed-cwd.

As a client of libcurl, it seems more intuitive when trying to upload a file
to say "put it there, no matter whether that directory exists before I
upload or not. Just do it." Clearly this would not be what you would want
by default, but I believe it adds real value to the library, much like the
ability to upload only if the file has changed (amazing work on that front,
guys! Works like a charm!).

> To add a new option, you could simple check one existing option and see
how it
> is set in the src/curl.c client code's curl_easy_setopt() using the
defines in
> include/curl/curl.h ending up in lib/url.c's Curl_setopt() big switch()
that
> stores the setting... And the setting is then typically a struct field in
the
> SessionHandle's sub-struct 'UserDefined'. As found in lib/urldata.h! :-)

Thanks! I'll take a look later today or possibly tomorrow morning; assuming
all goes well, I should have a new patch shortly thereafter, with
ftp_force_cwd as an option rather than a hard-coded patch.

> > I'm not sure if the ftp_free_dirs call that I added (as opposed to
simply
> > refactored in) is appropriate in the context of a non-reused connection
or a
> > download. It seems to work on my first connection just fine, but I
don't
> > have the time to write test cases to verify that it's downloading
> > appropriately.
>
> Well, does all the tests still run fine? If you can't write the test case
> yourself, can you at least provide enough information so that someone else
can
> write up one?

As I mentioned, I'm really a curl/libcurl newbie. This patch is extremely
peripheral to what I'm really trying to accomplish, so I haven't had the
time to validate it thoroughly nor to research *how* to validate it
thoroughly. If I can make some time I'll try to figure out how your test
system works and put the patch through its paces, but schedules are
excruciatingly tight these days and I can't promise much on that front.

--
-- Early Ehlinger CEO, ResPower Inc - Toll-Free : 866-737-7697
-- www.respower.com -- 500+ GHz Supercomputer Starting At USD$0.50/GHz*Hour
begin 666 ftp.diff
M+2TM(&-U<FPM-RXQ,"XV+VQI8B]F=' N8PE-;VX_at_075G(" T(#$W.C Q.C(T
M(#(P,#,**RLK(&]R:6<O8W5R;"TW+C$P+C8O;&EB+V9T<"YC"4UO;B!*=6P@
M,C@@,#,Z-3 Z,#(@,C P,PI 0" M,3 Q+#$T("LQ,#$L,3 @0$ *("\J($QO
M8V%L($%022!F=6YC=&EO;G,@*B\*('-T871I8R!#55),8V]D92!F='!?<V5N
M9'%U;W1E*'-T<G5C="!C;VYN96-T9&%T82 J8V]N;BP@<W1R=6-T(&-U<FQ?
M<VQI<W0@*G%U;W1E*3L*('-T871I8R!#55),8V]D92!F='!?8W=D*'-T<G5C
M="!C;VYN96-T9&%T82 J8V]N;BP_at_8VAA<B J<&%T:"D["BUS=&%T:6,@0U52
M3&-O9&4_at_9G1P7VUK9"AS=')U8W0_at_8V]N;F5C=&1A=&$@*F-O;FXL(&-H87(@
M*G!A=&@I.PHM<W1A=&EC($-54DQC;V1E(&9T<%]F;W)C95]C=V0H<W1R=6-T
M(&-O;FYE8W1D871A("IC;VYN+"!C:&%R("IP871H*3L*( H@+RH_at_96%S>2UT
M;RUU<V4@;6%C<F\Z("HO"B C9&5F:6YE($944%-%3D1&*'@L>2QZ*2!I9B_at_H
M<F5S=6QT(#T_at_0W5R;%]F='!S96YD9BAX+'DL>BDI*2!R971U<FX@<F5S=6QT
M"B *+79O:60_at_9G1P7V9R965?9&ER<R@@<W1R=6-T($944"H_at_9G1P("D["BT*
M("\J*BHJ*BHJ*BHJ*BHJ*BHJ*BHJ*BHJ*BHJ*BHJ*BHJ*BHJ*BHJ*BHJ*BHJ
M*BHJ*BHJ*BHJ*BHJ*BHJ*BHJ*BHJ*BHJ*BHJ*@H@("H*(" J($%L;&]W4V5R
M=F5R0V]N;F5C="@I"D! ("TQ.3_at_R+#$U("LQ.3<X+#$V($! "B @(" @('1R
M86YS9F5R(&ES('1A:VEN9R!P;&%C92P@=V4@;75S="!N;W<@9V5T(&)A8VL@
M=&\@=&AE(&]R:6=I;F%L(&1I<@H@(" @("!W:&5R92!W92!E;F1E9"!U<"!A
M9G1E<B!L;V=I;CH@*B\*(" @:68@*&-O;FXM/F)I=',N<F5U<V4@)B8_at_9G1P
M+3YE;G1R>7!A=&@I('L*+2 @("!I9B H*')E<W5L=" ](&9T<%]F;W)C95]C
M=V0H8V]N;BP_at_9G1P+3YE;G1R>7!A=&@I*2 A/2!#55),15]/2RD**R @("!I
M9B H*')E<W5L=" ](&9T<%]C=V0H8V]N;BP_at_9G1P+3YE;G1R>7!A=&@I*2 A
M/2!#55),15]/2RD*(" @(" @(')E='5R;B!R97-U;'0["B @('T*( H@("![
M"B @(" @:6YT(&D[("\J(&-O=6YT97(@9F]R(&QO;W @*B\*(" @("!F;W(@
M*&D],#L_at_9G1P+3YD:7)S6VE=.R!I*RLI('L*+2 @(" @(&EF("@H<F5S=6QT
M(#T_at_9G1P7V9O<F-E7V-W9"AC;VYN+"!F=' M/F1I<G-;:5TI*2 A/2!#55),
M15]/2R I"BT)<F5T=7)N(')E<W5L=#L**R @(" @("\J(%)&0R Q-S,X('-A
M>7,@96UP='D_at_8V]M<&]N96YT<R!S:&]U;&0_at_8F4@<F5S<&5C=&5D('1O;R J
M+PHK(" @(" @:68@*"AR97-U;'0@/2!F='!?8W=D*&-O;FXL(&9T<"T^9&ER
M<UMI72DI("$]($-54DQ%7T]+*0HK(" @(" @("!R971U<FX@<F5S=6QT.PH@
M(" @('T*(" @?0H@"D! ("TR,3,U+#@@*S(Q,S(L-B! 0 H@(" O*B @9FEX
M960@.B!I;FET:6%L:7IE(&9T<"T^9&ER<UMX>'A=('1O($Y53$P@(0H@(" @
M(" @:7,@9&]N92!I;B!#=7)L7V9T<%]C;VYN96-T*"D@*B\*( HM("!F='!?
M9G)E95]D:7)S*"!F=' @*3L*+0H@(" O*B!P87)S92!T:&4_at_55),('!A=&@@
M:6YT;R!S97!A<F%T92!P871H(&-O;7!O;F5N=',@*B\*(" @=VAI;&4H*'-L
M87-H7W!O<SUS=')C:'(H8W5R7W!O<RP@)R\G*2DI('L*(" @(" O*B Q(&]R
M(# @=&\@:6YD:6-A=&4_at_86)S;VQU=&4_at_9&ER96-T;W)Y("HO"D! ("TR,3<P
M+#<@*S(Q-C4L,3$@0$ *(" @(" @('T*(" @("!]"B @(" @:68@*')E=&-O
M9&4I('L*+2 @(" @(&9T<%]F<F5E7V1I<G,H(&9T<" I.PHK(" @(" @:6YT
M(&D["BL@(" @("!F;W(@*&D],#MI/'!A=&A?<&%R=#MI*RLI('L@+RH_at_9G)E
M92!P<F5V:6]U<R!P87)T<R J+PHK(" @(" @("!F<F5E*&9T<"T^9&ER<UMI
M72D["BL@(" @(" @(&9T<"T^9&ER<UMI73U.54Q,.PHK(" @(" @?0H@(" @
M(" @<F5T=7)N(')E=&-O9&4[("\J(&9A:6QU<F4@*B\*(" @("!]"B @('T*
M0$ @+3(Q.# L-R K,C$W.2PQ,2! 0 H@("!I9B_at_J9G1P+3YF:6QE*2!["B @
M(" @9G1P+3YF:6QE(#T_at_8W5R;%]U;F5S8V%P92AF=' M/F9I;&4L(# I.PH@
M(" @(&EF*$Y53$P@/3T_at_9G1P+3YF:6QE*2!["BT@(" @("!F='!?9G)E95]D
M:7)S*"!F=' @*3L**R @(" @(&EN="!I.PHK(" @(" @9F]R("AI/3 [:3QP
M871H7W!A<G0[:2LK*7L**R @(" @(" @9G)E92AF=' M/F1I<G-;:5TI.PHK
M(" @(" @("!F=' M/F1I<G-;:5T]3E5,3#L**R @(" @('T*(" @(" @(&9A
M:6QF*&1A=&$L(")N;R!M96UO<GDB*3L*(" @(" @(')E='5R;B!#55),15]/
M551?3T9?345-3U)9.PH@(" @('T*0$ @+3(R.#$L-C(@*S(R.#0L,30_at_0$ *
M(" @(" @(&9R964H9G1P+3YC86-H92D["B @(" @:68H9G1P+3YF:6QE*0H@
M(" @(" @9G)E92AF=' M/F9I;&4I.PHM(" @(&9T<%]F<F5E7V1I<G,H(&9T
M<" I.PHM"BT@(" @9G1P+3YF:6QE(#T_at_3E5,3#L@+RH@>F5R;R J+PHM("!]
M"BT@(')E='5R;B!#55),15]/2SL*+7T*+0HM=F]I9"!F='!?9G)E95]D:7)S
M*"!S=')U8W0_at_1E10*B!F=' @*0HM>PHM("!I;G0@:3L*+2 @9F]R*"!I/3 [
M9G1P+3YD:7)S6VE=.VDK*RD*+2 @("!["BT@(" @("!F<F5E*"!F=' M/F1I
M<G-;:5TI.PHK(" @(&9O<B H:3TP.V9T<"T^9&ER<UMI73MI*RLI>PHK(" @
M(" @9G)E92AF=' M/F1I<G-;:5TI.PH@(" @(" @9G1P+3YD:7)S6VE=/4Y5
M3$P["B @(" @?0HM?0HM"BU#55),8V]D92!F='!?;6MD*'-T<G5C="!C;VYN
M96-T9&%T82 J8V]N;BP_at_8VAA<B J<&%T:"D*+7L*+2 @0U523&-O9&4@<F5S
M=6QT/4-54DQ%7T]+.PHM("!I;G0_at_9G1P8V]D93L@+RH_at_9F]R(&9T<"!S=&%T
M=7,@*B\*+2 @<W-I>F5?="!N<F5A9#L*+2 @8VAA<B J8G5F(#T_at_8V]N;BT^
M9&%T82T^<W1A=&4N8G5F9F5R.PHM"BT@("\J($-R96%T92!A(&1I<F5C=&]R
M>2!O;B!T:&4@<F5M;W1E('-E<G9E<B J+PHM("!&5%!314Y$1BAC;VYN+" B
M34M$("5S(BP@<&%T:"D["BT*+2 @<F5S=6QT(#T_at_0W5R;%]'971&5%!297-P
M;VYS92_at_F;G)E860L(&-O;FXL("9F='!C;V1E*3L*+2 @:68H<F5S=6QT*0HM
M(" @(')E='5R;B!R97-U;'0["B *+2 @<W=I=&-H*&9T<&-O9&4I('L*+2 @
M8V%S92 R-3<Z"BT@(" @>PHM(" @(" @+RH@<W5C8V5S<R$@*B\*+2 @(" @
M(&EN9F]F*"!C;VYN+3YD871A("P@(D-R96%T960_at_4F5M;W1E($1I<F5C=&]R
M>2 E<UQN(B L('!A=&@@*3L*+2 @("!]"BT@(" @8G)E86L["BT@(&1E9F%U
M;'0Z"BT@(" @:6YF;V8H8V]N;BT^9&%T82P@(G5N<F5C;V=N:7IE9"!-2T0@
M<F5S<&]N<V4@)61<;B(L(')E<W5L=" I.PHM(" @(')E<W5L=" ]('Y#55),
M15]/2SL*+2 @("!B<F5A:SL**R @("!F=' M/F9I;&4@/2!.54Q,.R O*B!Z
M97)O("HO"B @('T*+2 @<F5T=7)N("!R97-U;'0["BU]"BT*+4-54DQC;V1E
M(&9T<%]F;W)C95]C=V0H<W1R=6-T(&-O;FYE8W1D871A("IC;VYN+"!C:&%R
M("IP871H("D*+7L*+2 @0U523&-O9&4@<F5S=6QT/4-54DQ%7T]+.PHM("!I
M9B H($-54DQ%7T]+("$]("@@<F5S=6QT(#T_at_9G1P7V-W9"@@8V]N;B L('!A
M=&@@*2 I("D*+2 @("!["BT@(" @("!I9B H($-54DQ%7T]+("$]("@@<F5S
M=6QT(#T_at_9G1P7VUK9"@@8V]N;B L('!A=&@@*2 I("D*+0ER971U<FX@<F5S
M=6QT.PHM(" @(" @<F5S=6QT(#T_at_9G1P7V-W9"@@8V]N;B L('!A=&@@*3L*
M+2 @("!]"BT@(')E='5R;B!R97-U;'0["BL@(')E='5R;B!#55),15]/2SL*
D('T*( H@(V5N9&EF("\J($-54DQ?1$E304),15]&5% @*B\*
`
end
-------------------------------------------------------
This SF.Net email sponsored by: Free pre-built ASP.NET sites including
Data Reports, E-commerce, Portals, and Forums are available now.
Download today and enter to win an XBOX or Visual Studio .NET.
http://aspnet.click-url.com/go/psa00100003ave/direct;at.aspnet_072303_01/01
Received on 2003-08-05