Hi Tim,

On Mon, 22 Jun 2020, Tim Johnson wrote:

> Hi Levente,
>
> On Jun 20, 2020, at 8:18 AM, Levente Uzonyi wrote:
>
>> On Fri, 19 Jun 2020, Tim Johnson wrote:
>> 
>>> I'm proposing a change to 
>>> WebMessage>>#streamDirectlyFrom:to:size:progress:
>>> ...but my proposed change could very well be wrong, or be poor logic.  So 
>>> I'm writing up my lengthy rationale below.
>>> What do you think about changing:
>>> [(sizeOrNil == nil and:[srcStream atEnd not]) or:[totalRead < size]] 
>>> whileTrue:[
>> 
>> I read the above line (combined with the assignment to size and totalRead) 
>> as: if no size was specified (sizeOrNil == nil, size = 0), then read while 
>> there's data available from the source stream (srcStream atEnd not).
>> If there was a size specified, assume that its the actual number of bytes 
>> available, and read that many bytes (totalRead < size).
>
> Ah!  Thank you.  That makes sense.
>
> I did not find any code path in which sizeOrNil might be nil, but I may not 
> have looked hard enough, or perhaps it was there for future expansion, etc.

It's used when you send a request to a server and the response does not 
contain the Content-Length header and the Transfer-Encoding is not 
chunked.

>
>> The last line is problematic, as you pointed out in your analysis, when the 
>> source stream and the size are both provided by the other party.
>> 
>>> to:
>>> [sizeOrNil == nil or: [totalRead < size and: [srcStream atEnd not]]] 
>>> whileTrue:[
>> 
>> The problem with the above is that it'll become and infinite loop if 
>> sizeOrNil is nil.
>
> Oops.
>
>> So, if you want to read while there's data available from the stream, then 
>> I think the following is the right choice:
>> 
>> [ srcStream atEnd not and: [ sizeOrNil isNil or: [ totalRead < sizeOrNil ] 
>> ] whileTrue: [
>> 
>> The above always takes into account whether there's data available from the 
>> stream and directly uses sizeOrNil for size comparison to make it more 
>> legible. However, there seem to be a few more things to clean up in that 
>> method.
>
> Thank you!  I have loaded WebClient-Core-ul.122.mcz from the Inbox into my 
> server image (running 5.2) and will see how things go.
>
> Strangely, the method's author is listed as MCPackageTest...?
>

Thanks. I had a look at MCPackageTest from the same image I changed the 
method. MCPackageTest is a subclass of MCTestCase which mangles with 
authorInitials. It seems the unwind code restoring authorInitials didn't 
execute.
I noticed the strange initials when I saved the package but I didn't think 
about the methods being affected as well.


Levente