The Inbox: Monticello-ul.727.mcz

>
>
> Best,
>
> Christoph
>
> _________________________________________________________________________________________________________________________________________________________________________________________________________________________________
> Von: Squeak-dev <[hidden email]> im Auftrag von [hidden email] <[hidden email]>
> Gesendet: Donnerstag, 17. September 2020 01:55:41
> An: [hidden email]
> Betreff: [squeak-dev] The Inbox: Monticello-ul.727.mcz  
> Levente Uzonyi uploaded a new version of Monticello to project The Inbox:
> http://source.squeak.org/inbox/Monticello-ul.727.mcz
>
> ==================== Summary ====================
>
> Name: Monticello-ul.727
> Author: ul
> Time: 17 September 2020, 1:54:51.056164 am
> UUID: ad776836-42eb-4aa2-b788-f10dd9e07da2
> Ancestors: Monticello-cmm.726
>
> MCHttpRepository changes:
> - before up- or downloading files, transform the urls using #rewriteUrl:forDownload:. The default rules (see #urlRewriteRules) switch from http to https for source.squeak.org and squeaksource.com, and switch to the the static
> smalltalkhub site for downloads. The url rewriting is Eliot's idea, but this implementation uses a list of rewrite rules instead of a dictionary-based mapping.
> - use WebClient (and the shared webclient instance) for uploads too
> - retry down/uploading with WebClient at most 3 times. This should work around the case where the underlying socket was closed but the state of the socket has not been updated in Squeak.
> - use https in #creationTemplate
>
> =============== Diff against Monticello-cmm.726 ===============
>
> Item was changed:
>   MCFileBasedRepository subclass: #MCHttpRepository
>          instanceVariableNames: 'location user password readerCache indexed webClient'
> +        classVariableNames: 'URLRewriteRules UseSharedWebClientInstance'
> -        classVariableNames: 'UseSharedWebClientInstance'
>          poolDictionaries: ''
>          category: 'Monticello-Repositories'!
>
> Item was changed:
>   ----- Method: MCHttpRepository class>>creationTemplate (in category 'ui-support') -----
>   creationTemplate
> +        ^self creationTemplateLocation: 'https://www.squeaksource.com/ProjectName'
> -        ^self creationTemplateLocation: 'http://www.squeaksource.com/ProjectName'
>                  user: 'squeak'
>                  password: 'squeak'
>   !
>
> Item was added:
> + ----- Method: MCHttpRepository class>>rewriteUrl:forDownload: (in category 'url rewrite') -----
> + rewriteUrl: aString forDownload: forDownload
> +
> +        | result |
> +        result := aString.
> +        self urlRewriteRules groupsDo: [ :regexString :replacement :downloadOnly |
> +                (forDownload or: [ downloadOnly not ])  ifTrue: [
> +                        result := result copyWithRegex: regexString matchesReplacedWith: replacement ] ].
> +        ^result
> +       
> + "
> + self assert:  'https://squeaksource.com/foo/bar?baz=1' = (self rewriteUrl: 'http://squeaksource.com/foo/bar?baz=1' forDownload: true).
> + self assert:  'https://squeaksource.com/foo/bar?baz=1' = (self rewriteUrl: 'https://squeaksource.com/foo/bar?baz=1' forDownload: true).
> + self assert:  'https://source.squeak.org/foo/bar?baz=1' = (self rewriteUrl: 'http://source.squeak.org/foo/bar?baz=1' forDownload: true).
> + self assert:  'https://source.squeak.org/foo/bar?baz=1' = (self rewriteUrl: 'https://source.squeak.org/foo/bar?baz=1' forDownload: true).
> + self assert:  'http://static.smalltalkhub.com/foo/bar?baz=1' = (self rewriteUrl: 'http://smalltalkhub.com/foo/bar?baz=1' forDownload: true).
> + self assert:  'http://smalltalkhub.com/foo/bar?baz=1' = (self rewriteUrl: 'http://smalltalkhub.com/foo/bar?baz=1' forDownload: false).
> + "!
>
> Item was added:
> + ----- Method: MCHttpRepository class>>urlRewriteRules (in category 'url rewrite') -----
> + urlRewriteRules
> +
> +        ^URLRewriteRules ifNil: [
> +                URLRewriteRules := #(
> +                        "Regex to be replaced"  "static replacement string"     "download only"
> +                        '^http\://source\.squeak\.org/' 'https://source.squeak.org/' false
> +                        '^http\://squeaksource\.com/' 'https://squeaksource.com/' false
> +                        '^http\://www.squeaksource\.com/' 'https://www.squeaksource.com/' false
> +                        '^http\://smalltalkhub.com/' 'http://static.smalltalkhub.com/' true    
> +                )  asOrderedCollection ]!
>
> Item was changed:
>   ----- Method: MCHttpRepository>>httpGet:arguments: (in category 'private') -----
>   httpGet: url arguments: arguments
>  
> +        | urlString |
> -        | progress urlString client  response result |
> -        progress := [ :total :amount |
> -                HTTPProgress new
> -                        total: total;
> -                        amount: amount;
> -                        signal: 'Downloading...' ].
>          urlString := arguments
>                  ifNil: [ url ]
>                  ifNotNil: [
>                          | queryString |
>                          queryString := WebUtils encodeUrlEncodedForm: arguments.
>                          (url includes: $?)
>                                  ifTrue: [ url, '&', queryString ]
>                                  ifFalse: [ url, '?', queryString ] ].
> +        urlString := self class rewriteUrl: urlString forDownload: true.
> +        ^self webClientDo: [ :client |
> +                client
> +                        username: self user;
> +                        password: self password;
> +                        httpGet: urlString do: [ :request |
> +                                request
> +                                        headerAt: 'Authorization' put: 'Basic ', (self user, ':', self password) base64Encoded;
> +                                        headerAt: 'Connection' put: 'Keep-Alive';
> +                                        headerAt: 'Accept' put: '*/*' ] ]!
> -        self class useSharedWebClientInstance ifTrue: [
> -                "Acquire webClient by atomically storing it in the client variable and setting its value to nil."
> -                client := webClient.
> -                webClient := nil ].
> -        client
> -                ifNil: [ client := WebClient new ]
> -                ifNotNil: [
> -                        "Attempt to avoid an error on windows by recreating the underlying stream."
> -                        client isConnected ifFalse: [ client close ] ].
> -        response := client
> -                username: self user;
> -                password: self password;
> -                httpGet: urlString do: [ :request |
> -                        request
> -                                headerAt: 'Authorization' put: 'Basic ', (self user, ':', self password) base64Encoded;
> -                                headerAt: 'Connection' put: 'Keep-Alive';
> -                                headerAt: 'Accept' put: '*/*' ].
> -        result := (response code between: 200 and: 299)
> -                ifFalse: [
> -                        response content. "Make sure content is read."
> -                        nil ]
> -                ifTrue: [ (RWBinaryOrTextStream with: (response contentWithProgress: progress)) reset ].
> -        self class useSharedWebClientInstance
> -                ifTrue: [
> -                        "Save the WebClient instance for reuse, but only if there is no client cached."
> -                        webClient 
> -                                ifNil: [ webClient := client ]
> -                                ifNotNil: [ client close ] ]
> -                ifFalse: [ client close ].
> -        result ifNil: [ NetworkError signal: 'Could not access ', location ].
> -        ^result!
>
> Item was added:
> + ----- Method: MCHttpRepository>>webClientDo: (in category 'private') -----
> + webClientDo: aBlock
> +
> +        | client attemptsLeft response result |
> +        self class useSharedWebClientInstance ifTrue: [
> +                "Acquire webClient by atomically storing it in the client variable and setting its value to nil."
> +                client := webClient.
> +                webClient := nil ].
> +       
> +        client
> +                ifNil: [ client := WebClient new ]
> +                ifNotNil: [
> +                        "Attempt to avoid an error by recreating the underlying stream."
> +                        client isConnected ifFalse: [ client close ] ].
> +               
> +        attemptsLeft := 3.
> +        response := nil.
> +        [ response isNil and: [ attemptsLeft > 0 ] ] whileTrue: [
> +                response := [ aBlock value: client ]
> +                        on: NetworkError
> +                        do: [ :error |
> +                                attemptsLeft = 0 ifTrue: [ error pass ].
> +                                (3 - attemptsLeft) seconds asDelay wait.
> +                                attemptsLeft := attemptsLeft - 1.
> +                                nil "The response" ] ].
> +       
> +        result := (response code between: 200 and: 299)
> +                ifFalse: [
> +                        response content. "Make sure content is read."
> +                        nil ]
> +                ifTrue: [
> +                        (RWBinaryOrTextStream with: (
> +                                response contentWithProgress:  [ :total :amount |
> +                                        HTTPProgress new
> +                                                total: total;
> +                                                amount: amount;
> +                                                signal ])) reset ].
> +
> +        self class useSharedWebClientInstance
> +                ifTrue: [
> +                        "Save the WebClient instance for reuse, but only if there is no client cached."
> +                        webClient 
> +                                ifNil: [ webClient := client ]
> +                                ifNotNil: [ client close ] ]
> +                ifFalse: [ client close ].
> +
> +        result ifNil: [ NetworkError signal: 'Could not access ', location ].
> +        ^result!
>
> Item was changed:
>   ----- Method: MCHttpRepository>>writeStreamForFileNamed:replace:do: (in category 'private') -----
>   writeStreamForFileNamed: aString replace: ignoreBoolean do: aBlock
> +
> +        | stream urlString |
> -        | stream response statusLine code |
>          stream := RWBinaryOrTextStream on: String new.
>          aBlock value: stream.
> +        urlString := self urlForFileNamed: aString.
> +        urlString := self class rewriteUrl: urlString forDownload: false.
> +        ^self displayProgress: 'Uploading ', aString during: [
> +                self webClientDo: [ :client |
> +                        client
> +                                username: self user;
> +                                password: self password;
> +                                httpPut: urlString
> +                                        content: stream contents
> +                                        type: nil
> +                                        do: [ :request |
> +                                                request
> +                                                        headerAt: 'Authorization' put: 'Basic ', (self user, ':', self password) base64Encoded;
> +                                                        headerAt: 'Connection' put: 'Keep-Alive';
> +                                                        headerAt: 'Accept' put: '*/*'  ] ] ]!
> -        self displayProgress: 'Uploading ', aString during:[
> -                response := HTTPSocket
> -                                        httpPut: stream contents
> -                                        to: (self urlForFileNamed: aString)
> -                                        user: self user
> -                                        passwd: self password.
> -        ].
> -        "More robust handling of HTTP responses. Instead of enumerating
> -        all possible return codes and http versions, do a quick parse"
> -        (response beginsWith: 'HTTP/') ifTrue:[
> -                "Looks like an HTTP header, not some error message"
> -                statusLine := response copyUpTo: Character cr.
> -                code := [(statusLine findTokens: ' ') second asInteger] on: Error do:[].
> -        ].
> -        (code isInteger and:[code between: 200 and: 299])
> -                ifFalse:[self error: response].!
>
>
>
>

>
> Hi Levente,
>
>
> alright, then please go ahead, I'm looking forward to seeing this in the Trunk! :-)
>
>
> Best,
>
> Christoph
>
> _________________________________________________________________________________________________________________________________________________________________________________________________________________________________
> Von: Squeak-dev <[hidden email]> im Auftrag von Levente Uzonyi <[hidden email]>
> Gesendet: Donnerstag, 17. September 2020 12:49:28
> An: The general-purpose Squeak developers list
> Betreff: Re: [squeak-dev] The Inbox: Monticello-ul.727.mcz  
> Hi Christoph,
>
> On Thu, 17 Sep 2020, Thiede, Christoph wrote:
>
> >
> > Very nice idea! :-) If I wanted to hijack an image, these URLRewriteRules would probably be my first approach - but security has never been an issue for Squeak, so I guess this is not a problem.
>
> If you can manipulate objects like URLRewriteRules, you have already
> hijacked the image.
>
> >
> >
> > Does this also fix the problem with the classic HTTP URLs returned by the update map or will we still need to patch them on the server side?
>
> The urls in the update map are used to create repositories. That's why
> simply changing the existing http repository urls to https doesn't
> suffice, because the updater will see the http urls and create new
> repositores with them if they are absent.
>
> So, this rewrite trick of Eliot works around the problem of the update
> maps as well.
>
> >
> >
> > And one last question regarding to your tests in the method comment of #rewriteUrl:forDownload:: Couldn't you put them into a real test case? I'm pretty sure that not everyone will run these out-commented tests manually,
> and
> > it would be a pity not to automate them.
>
> I left the possibility to change the rewrite rules to whatever you want.
> If there were a test case with those asserts in the comment, the test
> would start failing as soon as you changed the rules.
> Thought it's possible to create a test case which temporarily resets to
> the default rules and tests them.
>
>
> Levente
>
> >
> >
> > Best,
> >
> > Christoph
> >
> >________________________________________________________________________________________________________________________________________________________________________________________________________________________________
> _
> > Von: Squeak-dev <[hidden email]> im Auftrag von [hidden email] <[hidden email]>
> > Gesendet: Donnerstag, 17. September 2020 01:55:41
> > An: [hidden email]
> > Betreff: [squeak-dev] The Inbox: Monticello-ul.727.mcz  
> > Levente Uzonyi uploaded a new version of Monticello to project The Inbox:
> > http://source.squeak.org/inbox/Monticello-ul.727.mcz
> >
> > ==================== Summary ====================
> >
> > Name: Monticello-ul.727
> > Author: ul
> > Time: 17 September 2020, 1:54:51.056164 am
> > UUID: ad776836-42eb-4aa2-b788-f10dd9e07da2
> > Ancestors: Monticello-cmm.726
> >
> > MCHttpRepository changes:
> > - before up- or downloading files, transform the urls using #rewriteUrl:forDownload:. The default rules (see #urlRewriteRules) switch from http to https for source.squeak.org and squeaksource.com, and switch to the the
> static
> > smalltalkhub site for downloads. The url rewriting is Eliot's idea, but this implementation uses a list of rewrite rules instead of a dictionary-based mapping.
> > - use WebClient (and the shared webclient instance) for uploads too
> > - retry down/uploading with WebClient at most 3 times. This should work around the case where the underlying socket was closed but the state of the socket has not been updated in Squeak.
> > - use https in #creationTemplate
> >
> > =============== Diff against Monticello-cmm.726 ===============
> >
> > Item was changed:
> >   MCFileBasedRepository subclass: #MCHttpRepository
> >          instanceVariableNames: 'location user password readerCache indexed webClient'
> > +        classVariableNames: 'URLRewriteRules UseSharedWebClientInstance'
> > -        classVariableNames: 'UseSharedWebClientInstance'
> >          poolDictionaries: ''
> >          category: 'Monticello-Repositories'!
> >
> > Item was changed:
> >   ----- Method: MCHttpRepository class>>creationTemplate (in category 'ui-support') -----
> >   creationTemplate
> > +        ^self creationTemplateLocation: 'https://www.squeaksource.com/ProjectName'
> > -        ^self creationTemplateLocation: 'http://www.squeaksource.com/ProjectName'
> >                  user: 'squeak'
> >                  password: 'squeak'
> >   !
> >
> > Item was added:
> > + ----- Method: MCHttpRepository class>>rewriteUrl:forDownload: (in category 'url rewrite') -----
> > + rewriteUrl: aString forDownload: forDownload
> > +
> > +        | result |
> > +        result := aString.
> > +        self urlRewriteRules groupsDo: [ :regexString :replacement :downloadOnly |
> > +                (forDownload or: [ downloadOnly not ])  ifTrue: [
> > +                        result := result copyWithRegex: regexString matchesReplacedWith: replacement ] ].
> > +        ^result
> > +       
> > + "
> > + self assert:  'https://squeaksource.com/foo/bar?baz=1' = (self rewriteUrl: 'http://squeaksource.com/foo/bar?baz=1' forDownload: true).
> > + self assert:  'https://squeaksource.com/foo/bar?baz=1' = (self rewriteUrl: 'https://squeaksource.com/foo/bar?baz=1' forDownload: true).
> > + self assert:  'https://source.squeak.org/foo/bar?baz=1' = (self rewriteUrl: 'http://source.squeak.org/foo/bar?baz=1' forDownload: true).
> > + self assert:  'https://source.squeak.org/foo/bar?baz=1' = (self rewriteUrl: 'https://source.squeak.org/foo/bar?baz=1' forDownload: true).
> > + self assert:  'http://static.smalltalkhub.com/foo/bar?baz=1' = (self rewriteUrl: 'http://smalltalkhub.com/foo/bar?baz=1' forDownload: true).
> > + self assert:  'http://smalltalkhub.com/foo/bar?baz=1' = (self rewriteUrl: 'http://smalltalkhub.com/foo/bar?baz=1' forDownload: false).
> > + "!
> >
> > Item was added:
> > + ----- Method: MCHttpRepository class>>urlRewriteRules (in category 'url rewrite') -----
> > + urlRewriteRules
> > +
> > +        ^URLRewriteRules ifNil: [
> > +                URLRewriteRules := #(
> > +                        "Regex to be replaced"  "static replacement string"     "download only"
> > +                        '^http\://source\.squeak\.org/' 'https://source.squeak.org/' false
> > +                        '^http\://squeaksource\.com/' 'https://squeaksource.com/' false
> > +                        '^http\://www.squeaksource\.com/' 'https://www.squeaksource.com/' false
> > +                        '^http\://smalltalkhub.com/' 'http://static.smalltalkhub.com/' true    
> > +                )  asOrderedCollection ]!
> >
> > Item was changed:
> >   ----- Method: MCHttpRepository>>httpGet:arguments: (in category 'private') -----
> >   httpGet: url arguments: arguments
> >  
> > +        | urlString |
> > -        | progress urlString client  response result |
> > -        progress := [ :total :amount |
> > -                HTTPProgress new
> > -                        total: total;
> > -                        amount: amount;
> > -                        signal: 'Downloading...' ].
> >          urlString := arguments
> >                  ifNil: [ url ]
> >                  ifNotNil: [
> >                          | queryString |
> >                          queryString := WebUtils encodeUrlEncodedForm: arguments.
> >                          (url includes: $?)
> >                                  ifTrue: [ url, '&', queryString ]
> >                                  ifFalse: [ url, '?', queryString ] ].
> > +        urlString := self class rewriteUrl: urlString forDownload: true.
> > +        ^self webClientDo: [ :client |
> > +                client
> > +                        username: self user;
> > +                        password: self password;
> > +                        httpGet: urlString do: [ :request |
> > +                                request
> > +                                        headerAt: 'Authorization' put: 'Basic ', (self user, ':', self password) base64Encoded;
> > +                                        headerAt: 'Connection' put: 'Keep-Alive';
> > +                                        headerAt: 'Accept' put: '*/*' ] ]!
> > -        self class useSharedWebClientInstance ifTrue: [
> > -                "Acquire webClient by atomically storing it in the client variable and setting its value to nil."
> > -                client := webClient.
> > -                webClient := nil ].
> > -        client
> > -                ifNil: [ client := WebClient new ]
> > -                ifNotNil: [
> > -                        "Attempt to avoid an error on windows by recreating the underlying stream."
> > -                        client isConnected ifFalse: [ client close ] ].
> > -        response := client
> > -                username: self user;
> > -                password: self password;
> > -                httpGet: urlString do: [ :request |
> > -                        request
> > -                                headerAt: 'Authorization' put: 'Basic ', (self user, ':', self password) base64Encoded;
> > -                                headerAt: 'Connection' put: 'Keep-Alive';
> > -                                headerAt: 'Accept' put: '*/*' ].
> > -        result := (response code between: 200 and: 299)
> > -                ifFalse: [
> > -                        response content. "Make sure content is read."
> > -                        nil ]
> > -                ifTrue: [ (RWBinaryOrTextStream with: (response contentWithProgress: progress)) reset ].
> > -        self class useSharedWebClientInstance
> > -                ifTrue: [
> > -                        "Save the WebClient instance for reuse, but only if there is no client cached."
> > -                        webClient 
> > -                                ifNil: [ webClient := client ]
> > -                                ifNotNil: [ client close ] ]
> > -                ifFalse: [ client close ].
> > -        result ifNil: [ NetworkError signal: 'Could not access ', location ].
> > -        ^result!
> >
> > Item was added:
> > + ----- Method: MCHttpRepository>>webClientDo: (in category 'private') -----
> > + webClientDo: aBlock
> > +
> > +        | client attemptsLeft response result |
> > +        self class useSharedWebClientInstance ifTrue: [
> > +                "Acquire webClient by atomically storing it in the client variable and setting its value to nil."
> > +                client := webClient.
> > +                webClient := nil ].
> > +       
> > +        client
> > +                ifNil: [ client := WebClient new ]
> > +                ifNotNil: [
> > +                        "Attempt to avoid an error by recreating the underlying stream."
> > +                        client isConnected ifFalse: [ client close ] ].
> > +               
> > +        attemptsLeft := 3.
> > +        response := nil.
> > +        [ response isNil and: [ attemptsLeft > 0 ] ] whileTrue: [
> > +                response := [ aBlock value: client ]
> > +                        on: NetworkError
> > +                        do: [ :error |
> > +                                attemptsLeft = 0 ifTrue: [ error pass ].
> > +                                (3 - attemptsLeft) seconds asDelay wait.
> > +                                attemptsLeft := attemptsLeft - 1.
> > +                                nil "The response" ] ].
> > +       
> > +        result := (response code between: 200 and: 299)
> > +                ifFalse: [
> > +                        response content. "Make sure content is read."
> > +                        nil ]
> > +                ifTrue: [
> > +                        (RWBinaryOrTextStream with: (
> > +                                response contentWithProgress:  [ :total :amount |
> > +                                        HTTPProgress new
> > +                                                total: total;
> > +                                                amount: amount;
> > +                                                signal ])) reset ].
> > +
> > +        self class useSharedWebClientInstance
> > +                ifTrue: [
> > +                        "Save the WebClient instance for reuse, but only if there is no client cached."
> > +                        webClient 
> > +                                ifNil: [ webClient := client ]
> > +                                ifNotNil: [ client close ] ]
> > +                ifFalse: [ client close ].
> > +
> > +        result ifNil: [ NetworkError signal: 'Could not access ', location ].
> > +        ^result!
> >
> > Item was changed:
> >   ----- Method: MCHttpRepository>>writeStreamForFileNamed:replace:do: (in category 'private') -----
> >   writeStreamForFileNamed: aString replace: ignoreBoolean do: aBlock
> > +
> > +        | stream urlString |
> > -        | stream response statusLine code |
> >          stream := RWBinaryOrTextStream on: String new.
> >          aBlock value: stream.
> > +        urlString := self urlForFileNamed: aString.
> > +        urlString := self class rewriteUrl: urlString forDownload: false.
> > +        ^self displayProgress: 'Uploading ', aString during: [
> > +                self webClientDo: [ :client |
> > +                        client
> > +                                username: self user;
> > +                                password: self password;
> > +                                httpPut: urlString
> > +                                        content: stream contents
> > +                                        type: nil
> > +                                        do: [ :request |
> > +                                                request
> > +                                                        headerAt: 'Authorization' put: 'Basic ', (self user, ':', self password) base64Encoded;
> > +                                                        headerAt: 'Connection' put: 'Keep-Alive';
> > +                                                        headerAt: 'Accept' put: '*/*'  ] ] ]!
> > -        self displayProgress: 'Uploading ', aString during:[
> > -                response := HTTPSocket
> > -                                        httpPut: stream contents
> > -                                        to: (self urlForFileNamed: aString)
> > -                                        user: self user
> > -                                        passwd: self password.
> > -        ].
> > -        "More robust handling of HTTP responses. Instead of enumerating
> > -        all possible return codes and http versions, do a quick parse"
> > -        (response beginsWith: 'HTTP/') ifTrue:[
> > -                "Looks like an HTTP header, not some error message"
> > -                statusLine := response copyUpTo: Character cr.
> > -                code := [(statusLine findTokens: ' ') second asInteger] on: Error do:[].
> > -        ].
> > -        (code isInteger and:[code between: 200 and: 299])
> > -                ifFalse:[self error: response].!
> >
> >
> >
> >
>
>

>
>
> Levente
>
> On Sat, 19 Sep 2020, Thiede, Christoph wrote:
>
>>
>> Hi Levente,
>>
>>
>> alright, then please go ahead, I'm looking forward to seeing this in the
>> Trunk! :-)
>>
>>
>> Best,
>>
>> Christoph
>>
>> _________________________________________________________________________________________________________________________________________________________________________________________________________________________________
>> Von: Squeak-dev <[hidden email]> im Auftrag
>> von Levente Uzonyi <[hidden email]>
>> Gesendet: Donnerstag, 17. September 2020 12:49:28
>> An: The general-purpose Squeak developers list
>> Betreff: Re: [squeak-dev] The Inbox: Monticello-ul.727.mcz  
>> Hi Christoph,
>>
>> On Thu, 17 Sep 2020, Thiede, Christoph wrote:
>>
>> >
>> > Very nice idea! :-) If I wanted to hijack an image, these URLRewriteRules
>> would probably be my first approach - but security has never been an issue
>> for Squeak, so I guess this is not a problem.
>>
>> If you can manipulate objects like URLRewriteRules, you have already
>> hijacked the image.
>>
>> >
>> >
>> > Does this also fix the problem with the classic HTTP URLs returned by the
>> update map or will we still need to patch them on the server side?
>>
>> The urls in the update map are used to create repositories. That's why
>> simply changing the existing http repository urls to https doesn't
>> suffice, because the updater will see the http urls and create new
>> repositores with them if they are absent.
>>
>> So, this rewrite trick of Eliot works around the problem of the update
>> maps as well.
>>
>> >
>> >
>> > And one last question regarding to your tests in the method comment of
>> #rewriteUrl:forDownload:: Couldn't you put them into a real test case? I'm
>> pretty sure that not everyone will run these out-commented tests manually,
>> and
>> > it would be a pity not to automate them.
>>
>> I left the possibility to change the rewrite rules to whatever you want.
>> If there were a test case with those asserts in the comment, the test
>> would start failing as soon as you changed the rules.
>> Thought it's possible to create a test case which temporarily resets to
>> the default rules and tests them.
>>
>>
>> Levente
>>
>> >
>> >
>> > Best,
>> >
>> > Christoph
>> >
>> >________________________________________________________________________________________________________________________________________________________________________________________________________________________________
>> _
>> > Von: Squeak-dev <[hidden email]> im
>> Auftrag von [hidden email] <[hidden email]>
>> > Gesendet: Donnerstag, 17. September 2020 01:55:41
>> > An: [hidden email]
>> > Betreff: [squeak-dev] The Inbox: Monticello-ul.727.mcz  
>> > Levente Uzonyi uploaded a new version of Monticello to project The Inbox:
>> > http://source.squeak.org/inbox/Monticello-ul.727.mcz
>> >
>> > ==================== Summary ====================
>> >
>> > Name: Monticello-ul.727
>> > Author: ul
>> > Time: 17 September 2020, 1:54:51.056164 am
>> > UUID: ad776836-42eb-4aa2-b788-f10dd9e07da2
>> > Ancestors: Monticello-cmm.726
>> >
>> > MCHttpRepository changes:
>> > - before up- or downloading files, transform the urls using
>> #rewriteUrl:forDownload:. The default rules (see #urlRewriteRules) switch
>> from http to https for source.squeak.org and squeaksource.com, and switch
>> to the the
>> static
>> > smalltalkhub site for downloads. The url rewriting is Eliot's idea, but
>> this implementation uses a list of rewrite rules instead of a
>> dictionary-based mapping.
>> > - use WebClient (and the shared webclient instance) for uploads too
>> > - retry down/uploading with WebClient at most 3 times. This should work
>> around the case where the underlying socket was closed but the state of the
>> socket has not been updated in Squeak.
>> > - use https in #creationTemplate
>> >
>> > =============== Diff against Monticello-cmm.726 ===============
>> >
>> > Item was changed:
>> >   MCFileBasedRepository subclass: #MCHttpRepository
>> >          instanceVariableNames: 'location user password readerCache
>> indexed webClient'
>> > +        classVariableNames: 'URLRewriteRules UseSharedWebClientInstance'
>> > -        classVariableNames: 'UseSharedWebClientInstance'
>> >          poolDictionaries: ''
>> >          category: 'Monticello-Repositories'!
>> >
>> > Item was changed:
>> >   ----- Method: MCHttpRepository class>>creationTemplate (in category
>> 'ui-support') -----
>> >   creationTemplate
>> > +        ^self creationTemplateLocation:
>> 'https://www.squeaksource.com/ProjectName'
>> > -        ^self creationTemplateLocation:
>> 'http://www.squeaksource.com/ProjectName'
>> >                  user: 'squeak'
>> >                  password: 'squeak'
>> >   !
>> >
>> > Item was added:
>> > + ----- Method: MCHttpRepository class>>rewriteUrl:forDownload: (in
>> category 'url rewrite') -----
>> > + rewriteUrl: aString forDownload: forDownload
>> > +
>> > +        | result |
>> > +        result := aString.
>> > +        self urlRewriteRules groupsDo: [ :regexString :replacement
>> :downloadOnly |
>> > +                (forDownload or: [ downloadOnly not ])  ifTrue: [
>> > +                        result := result copyWithRegex: regexString
>> matchesReplacedWith: replacement ] ].
>> > +        ^result
>> > +       
>> > + "
>> > + self assert:  'https://squeaksource.com/foo/bar?baz=1' = (self
>> rewriteUrl: 'http://squeaksource.com/foo/bar?baz=1' forDownload: true).
>> > + self assert:  'https://squeaksource.com/foo/bar?baz=1' = (self
>> rewriteUrl: 'https://squeaksource.com/foo/bar?baz=1' forDownload: true).
>> > + self assert:  'https://source.squeak.org/foo/bar?baz=1' = (self
>> rewriteUrl: 'http://source.squeak.org/foo/bar?baz=1' forDownload: true).
>> > + self assert:  'https://source.squeak.org/foo/bar?baz=1' = (self
>> rewriteUrl: 'https://source.squeak.org/foo/bar?baz=1' forDownload: true).
>> > + self assert:  'http://static.smalltalkhub.com/foo/bar?baz=1' = (self
>> rewriteUrl: 'http://smalltalkhub.com/foo/bar?baz=1' forDownload: true).
>> > + self assert:  'http://smalltalkhub.com/foo/bar?baz=1' = (self
>> rewriteUrl: 'http://smalltalkhub.com/foo/bar?baz=1' forDownload: false).
>> > + "!
>> >
>> > Item was added:
>> > + ----- Method: MCHttpRepository class>>urlRewriteRules (in category 'url
>> rewrite') -----
>> > + urlRewriteRules
>> > +
>> > +        ^URLRewriteRules ifNil: [
>> > +                URLRewriteRules := #(
>> > +                        "Regex to be replaced"  "static replacement
>> string"     "download only"
>> > +                        '^http\://source\.squeak\.org/'
>> 'https://source.squeak.org/' false
>> > +                        '^http\://squeaksource\.com/'
>> 'https://squeaksource.com/' false
>> > +                        '^http\://www.squeaksource\.com/'
>> 'https://www.squeaksource.com/' false
>> > +                        '^http\://smalltalkhub.com/'
>> 'http://static.smalltalkhub.com/' true    
>> > +                )  asOrderedCollection ]!
>> >
>> > Item was changed:
>> >   ----- Method: MCHttpRepository>>httpGet:arguments: (in category
>> 'private') -----
>> >   httpGet: url arguments: arguments
>> >  
>> > +        | urlString |
>> > -        | progress urlString client  response result |
>> > -        progress := [ :total :amount |
>> > -                HTTPProgress new
>> > -                        total: total;
>> > -                        amount: amount;
>> > -                        signal: 'Downloading...' ].
>> >          urlString := arguments
>> >                  ifNil: [ url ]
>> >                  ifNotNil: [
>> >                          | queryString |
>> >                          queryString := WebUtils encodeUrlEncodedForm:
>> arguments.
>> >                          (url includes: $?)
>> >                                  ifTrue: [ url, '&', queryString ]
>> >                                  ifFalse: [ url, '?', queryString ] ].
>> > +        urlString := self class rewriteUrl: urlString forDownload: true.
>> > +        ^self webClientDo: [ :client |
>> > +                client
>> > +                        username: self user;
>> > +                        password: self password;
>> > +                        httpGet: urlString do: [ :request |
>> > +                                request
>> > +                                        headerAt: 'Authorization' put:
>> 'Basic ', (self user, ':', self password) base64Encoded;
>> > +                                        headerAt: 'Connection' put:
>> 'Keep-Alive';
>> > +                                        headerAt: 'Accept' put: '*/*' ]
>> ]!
>> > -        self class useSharedWebClientInstance ifTrue: [
>> > -                "Acquire webClient by atomically storing it in the
>> client variable and setting its value to nil."
>> > -                client := webClient.
>> > -                webClient := nil ].
>> > -        client
>> > -                ifNil: [ client := WebClient new ]
>> > -                ifNotNil: [
>> > -                        "Attempt to avoid an error on windows by
>> recreating the underlying stream."
>> > -                        client isConnected ifFalse: [ client close ] ].
>> > -        response := client
>> > -                username: self user;
>> > -                password: self password;
>> > -                httpGet: urlString do: [ :request |
>> > -                        request
>> > -                                headerAt: 'Authorization' put: 'Basic ',
>> (self user, ':', self password) base64Encoded;
>> > -                                headerAt: 'Connection' put:
>> 'Keep-Alive';
>> > -                                headerAt: 'Accept' put: '*/*' ].
>> > -        result := (response code between: 200 and: 299)
>> > -                ifFalse: [
>> > -                        response content. "Make sure content is read."
>> > -                        nil ]
>> > -                ifTrue: [ (RWBinaryOrTextStream with: (response
>> contentWithProgress: progress)) reset ].
>> > -        self class useSharedWebClientInstance
>> > -                ifTrue: [
>> > -                        "Save the WebClient instance for reuse, but only
>> if there is no client cached."
>> > -                        webClient 
>> > -                                ifNil: [ webClient := client ]
>> > -                                ifNotNil: [ client close ] ]
>> > -                ifFalse: [ client close ].
>> > -        result ifNil: [ NetworkError signal: 'Could not access ',
>> location ].
>> > -        ^result!
>> >
>> > Item was added:
>> > + ----- Method: MCHttpRepository>>webClientDo: (in category 'private')
>> -----
>> > + webClientDo: aBlock
>> > +
>> > +        | client attemptsLeft response result |
>> > +        self class useSharedWebClientInstance ifTrue: [
>> > +                "Acquire webClient by atomically storing it in the
>> client variable and setting its value to nil."
>> > +                client := webClient.
>> > +                webClient := nil ].
>> > +       
>> > +        client
>> > +                ifNil: [ client := WebClient new ]
>> > +                ifNotNil: [
>> > +                        "Attempt to avoid an error by recreating the
>> underlying stream."
>> > +                        client isConnected ifFalse: [ client close ] ].
>> > +               
>> > +        attemptsLeft := 3.
>> > +        response := nil.
>> > +        [ response isNil and: [ attemptsLeft > 0 ] ] whileTrue: [
>> > +                response := [ aBlock value: client ]
>> > +                        on: NetworkError
>> > +                        do: [ :error |
>> > +                                attemptsLeft = 0 ifTrue: [ error pass ].
>> > +                                (3 - attemptsLeft) seconds asDelay wait.
>> > +                                attemptsLeft := attemptsLeft - 1.
>> > +                                nil "The response" ] ].
>> > +       
>> > +        result := (response code between: 200 and: 299)
>> > +                ifFalse: [
>> > +                        response content. "Make sure content is read."
>> > +                        nil ]
>> > +                ifTrue: [
>> > +                        (RWBinaryOrTextStream with: (
>> > +                                response contentWithProgress:  [ :total
>> :amount |
>> > +                                        HTTPProgress new
>> > +                                                total: total;
>> > +                                                amount: amount;
>> > +                                                signal ])) reset ].
>> > +
>> > +        self class useSharedWebClientInstance
>> > +                ifTrue: [
>> > +                        "Save the WebClient instance for reuse, but only
>> if there is no client cached."
>> > +                        webClient 
>> > +                                ifNil: [ webClient := client ]
>> > +                                ifNotNil: [ client close ] ]
>> > +                ifFalse: [ client close ].
>> > +
>> > +        result ifNil: [ NetworkError signal: 'Could not access ',
>> location ].
>> > +        ^result!
>> >
>> > Item was changed:
>> >   ----- Method: MCHttpRepository>>writeStreamForFileNamed:replace:do: (in
>> category 'private') -----
>> >   writeStreamForFileNamed: aString replace: ignoreBoolean do: aBlock
>> > +
>> > +        | stream urlString |
>> > -        | stream response statusLine code |
>> >          stream := RWBinaryOrTextStream on: String new.
>> >          aBlock value: stream.
>> > +        urlString := self urlForFileNamed: aString.
>> > +        urlString := self class rewriteUrl: urlString forDownload:
>> false.
>> > +        ^self displayProgress: 'Uploading ', aString during: [
>> > +                self webClientDo: [ :client |
>> > +                        client
>> > +                                username: self user;
>> > +                                password: self password;
>> > +                                httpPut: urlString
>> > +                                        content: stream contents
>> > +                                        type: nil
>> > +                                        do: [ :request |
>> > +                                                request
>> > +                                                        headerAt:
>> 'Authorization' put: 'Basic ', (self user, ':', self password)
>> base64Encoded;
>> > +                                                        headerAt:
>> 'Connection' put: 'Keep-Alive';
>> > +                                                        headerAt:
>> 'Accept' put: '*/*'  ] ] ]!
>> > -        self displayProgress: 'Uploading ', aString during:[
>> > -                response := HTTPSocket
>> > -                                        httpPut: stream contents
>> > -                                        to: (self urlForFileNamed:
>> aString)
>> > -                                        user: self user
>> > -                                        passwd: self password.
>> > -        ].
>> > -        "More robust handling of HTTP responses. Instead of enumerating
>> > -        all possible return codes and http versions, do a quick parse"
>> > -        (response beginsWith: 'HTTP/') ifTrue:[
>> > -                "Looks like an HTTP header, not some error message"
>> > -                statusLine := response copyUpTo: Character cr.
>> > -                code := [(statusLine findTokens: ' ') second asInteger]
>> on: Error do:[].
>> > -        ].
>> > -        (code isInteger and:[code between: 200 and: 299])
>> > -                ifFalse:[self error: response].!
>> >
>> >
>> >
>> >
>>
>>
>