[squeak-dev] WebClient test fails in trunk

Andreas Raab Andreas.Raab at gmx.de
Mon Jul 16 14:03:44 UTC 2012


Hi David -

The old IPv6 code. Sigh. Unfortunately, this code was written with the idea 
of being a complete rewrite and not an add-on to the existing library but 
it's now used as an add-on and this causes all sorts of problems, including 
major compatibility issues.

I really do think that code needs serious overhaul. It basically needs to 
be rewritten such that:
a) The external interface remains simply a byte array (be that an IPv4 
style 4-element byte array or some sort of SocketAddress which really *is* 
a socket address and not some plugin-internal structure combining address, 
port, family, and the phase of the moon)
b) SocketAddress[Info] is used privately within the socket and never 
exposed to a client unless that client specifically asks for it (but 
definitely not via stuff like Socket>>[local|remote]Address)
c) NetNameResolver needs to return IPv4 addresses by default and only 
answer IPv6 if the client specifically requests it.

Without this you're basically playing whack-a-mole trying to catch bugs 
that pop up randomly. A straightforward user like WebClient should be able 
to use Sockets without any change, defaulting to IPv4, and only when it 
specifically requests IPv6 support should it be exposed to IPv6. Otherwise 
this will be a complete nightmare.

Cheers,
  - Andreas


>             
> -------- Original-Nachricht --------
> Datum: Sun, 15 Jul 2012 14:53:59 -0400
> Von: "David T. Lewis" <lewis at mail.msen.com>
> An: Andreas Raab <Andreas.Raab at gmx.de>
> CC: squeak-dev at lists.squeakfoundation.org
> Betreff: Re: [squeak-dev] WebClient test fails in trunk
> 
>             Hi Andreas,
> 
> The changes should hopefully work with any image regardless of whether
> the new socket code is present, and regardless of what flavor of VM and
> SocketPlugin are running.
> 
> There should be no compatibity issues either for IPv4 or IPv6. The only
> problem was that if the four-byte socket address is being represented by
> a SocketAddress (subclass of ByteArray) rather than a ByteArray, then
> converting it to a host name string with this does not work:
> 
>   String streamContents:[:s| anAddress
>     do:[:b| s print: b] separatedBy:[s nextPut: $.]]
> 
> Aside from that, no real issues as far as I know (other than the trivial
> undeclared CrLf issue).
> 
> A SocketAddress is a kind of ByteArray, but it represents an opaque 
> reference to a data structure managed in the plugin. That data structure
> does include the four-byte network address (for IPv4) but this should
> not be directly referenced from the image.
> 
> For example, on my PC with a 64 bit VM, the local address 127.0.0.2
> is represented by a SocketAddress that, if printed out byte-by-byte,
> would look like this:
> 
>  '117.49.82.81.16.0.0.0.2.0.0.0.127.0.0.2.0.0.0.0.0.0.0.0'
> 
> This is an 8 byte pointer, followed by the 32 bit network address, 
> followed
> by some other stuff I don't remember. But the printString for this object
> provides a more useful representation:
> 
>  '127.0.0.2(linux-jh8m),0(0)'
> 
> Thus for a network address represented in the traditional way, printing
> it out as bytes separated by $. does the right thing, and for the same
> address represented by a SocketAddress, the first portion of the 
> printString
> up to $( is suitable. I did not check to see if this works with an IPv6
> address, but it's good enough to cover the cases in the WebClient tests. 
> 
> I am guessing that for logging purposes, you would want to use the 
> complete
> SocketAddress>>printString '127.0.0.2(linux-jh8m),0(0)' while for 
> converting
> a socket address to a host string you would want only the address portion
> '127.0.0.2', so I put two separate methods in WebUtils for those two 
> cases. 
> 
> Dave
> 
> On Sun, Jul 15, 2012 at 06:48:15PM +0200, Andreas Raab wrote:
> > Hi David -
> >  
> >  Are these patches needed only for IPv6 or also to support IPv4? If the 
>  
> > latter, this would bother me greatly. I can understand why IPv6 may be  
> > implemented in incompatible ways but I can't see why there should be an 
>  
> > incompatible interface to IPv4 addresses which have been used for  
> eternity 
> > in Squeak. In particular given that (I believe) SocketAddress is a 
> subclass 
> > of ByteArray it strikes me as not particularly hard to have Sockets 
> support 
> > ByteArrays of length 4 as IPv4 socket addresses (and perhaps return 
> > instances of SocketAddress with length 4 or so). In any case, I'm not 
> > looking forward to seeing every last bit of networking code break - 
> there 
> > has to be a better way...
> > 
> >  Cheers,
> >  ?? - Andreas
> > 
> >  
> > >             
> > > -------- Original-Nachricht --------
> > > Datum: Sun, 15 Jul 2012 11:46:54 -0400
> > > Von: "David T. Lewis" <lewis at mail.msen.com>
> > > An: The general-purpose Squeak developers list 
> > > <squeak-dev at lists.squeakfoundation.org>, andreas.raab at gmx.de
> > > Betreff: Re: [squeak-dev] WebClient test fails in trunk
> > > 
> > >             On Wed, Jul 04, 2012 at 11:46:58PM +0200, Levente Uzonyi 
> > > wrote:
> > > > On Tue, 3 Jul 2012, Frank Shearar wrote:
> > > > 
> > > > >On 3 July 2012 17:42, Frank Shearar <frank.shearar at gmail.com> 
> wrote:
> > > > >>On 3 July 2012 17:38, Levente Uzonyi <leves at elte.hu> wrote:
> > > > >>>On Tue, 3 Jul 2012, Frank Shearar wrote:
> > > > >>>
> > > > >>>>Hi,
> > > > >>>>
> > > > >>>>I took WebClient for a spin in a trunk image, and found a 
> failing
> > > > >>>>test: WebClientServerTest >> #testServerDestroy throws a
> > > > >>>>"ConnectionClosed: Connection closed while waiting for data". 
> That
> > > > >>>>sounds familiar - I think this was introduced in a recent 
> Network
> > > > >>>>change.
> > > > >>>>
> > > > >>>>Still, it means WebClient needs fixing. I'll try fix it on my 
> > > evening
> > > > >>>>commute, but if anyone feels like racing me, I'll be happy!
> > > > >>>
> > > > >>>
> > > > >>>Interesting, I had no issue with it on windows, except for some 
> > > extension
> > > > >>>methods trying to use the CrLf class variable in HTTPSocket (or 
> some 
> > > > >>>other
> > > > >>>class), but that variable doesn't exist.
> > > > >>
> > > > >>Which reminds me of an important fact I should mention: I'm 
> testing
> > > > >>this on an Ubuntu Lynx box.
> > > > 
> > > > The VM also matters in this case. I tried it with an older CogVM 
> which 
> > > > doesn't have the new socket primitives.
> > > 
> > > I wanted to check that the recent network updates were not causing
> > > problems for WebClient. I found that some updates to WebClient are
> > > needed to account for the fact that network addresses are now being
> > > represented as SocketAddress rather than ByteArray when running on
> > > a VM that supports the new network primitives.
> > > 
> > > Attached is a change set that addresses all the issues that I could
> > > find. I'm assuming that the testServerDestroy issue discussed above
> > > has already been addressed, as I had no failures for that test.
> > > 
> > > From the preamble:
> > > 
> > >   Change Set:		WebClient-networkPatches-dtl
> > >   Date:			15 July 2012
> > >   Author:			David T Lewis
> > >   
> > >   These patches update WebClient to work with new network code in 
> Squeak
> > >   trunk (all tests green).
> > >   
> > >   Explanation: When using the new network code along with a VM that 
> > > supports the
> > >   IPV6 primitives, socket addresses are instances of SocketAddress 
> rather 
> > > than
> > >   ByteArray. Conversion of a socket address to a string must be 
> > > implemented
> > >   differently for SocketAddress.
> > >   
> > >   Also use String crlf instead of undeclared CrLf in HTTPSocket 
> > > class>>httpPostMultipart:args:accept:request:
> > >   
> > >   Note: Squeak automatically uses SocketAddress if the VM supports it 
> at 
> > > image
> > >   start time. To force use of old style ByteArray addresses, evaluate
> > >   NetNameResolver useOldNetwork: true.
> > >   
> > >   Follow up: The WebClient methods in HTTPSocket are marked as 
> overrides 
> > > (category
> > >   *WebClient-HTTP-override) but are not overrides with respect to 
> Squeak 
> > > trunk.
> > >   The override category prevents these methods from appearing in a 
> change 
> > > set
> > >   fileout. Should these methods just be adopted in trunk?
> > > 
> > > Dave
> > > 
> > > 
> >         
> 
        
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20120716/02d74000/attachment.htm


More information about the Squeak-dev mailing list