This repository has been archived by the owner. It is now read-only.

Fix installer/updater in platform for Joomla 3. Thanks Rouven #1479

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
4 participants
@javigomez
Contributor

javigomez commented Aug 21, 2012

When using the Updater in Joomla 3 right now you get a:

  • Notice: Undefined index: wrapper_data in /Users/rouven/Sites/joomla-cms/libraries/joomla/installer/helper.php on line 55

Checking with Rouven he suggested that change attached in the pull request. I have tested with the 3 transports like that:

// using STREAM
$http = new JHttp(null, new JHttpTransportStream(null));

// using sockets
$http = new JHttp(null, new JHttpTransportSocket(null));

// using CURL
$http = JHttpFactory::getHttp();

All they worked for me.

Please review it.
Thanks ^_^

@javigomez

This comment has been minimized.

Show comment
Hide comment
@javigomez

javigomez Aug 21, 2012

Contributor

Sorry, I made a mistake during the test. Actually is not working for Stream and Sockets. Only for CURL

Contributor

javigomez commented Aug 21, 2012

Sorry, I made a mistake during the test. Actually is not working for Stream and Sockets. Only for CURL

@javigomez javigomez closed this Aug 21, 2012

@elinw

This comment has been minimized.

Show comment
Hide comment
@elinw

elinw Aug 22, 2012

I'm wondering here first why Content-Disposition would be found outside of the wrapper_data. Would it possibly be better to check if $response->headers['wrapper_data']['Content-Disposition']) is !empty? Since it's optional it's valid for it to not be set or to be empty.
http://www.faqs.org/rfcs/rfc2183.html

elinw commented on 8120468 Aug 22, 2012

I'm wondering here first why Content-Disposition would be found outside of the wrapper_data. Would it possibly be better to check if $response->headers['wrapper_data']['Content-Disposition']) is !empty? Since it's optional it's valid for it to not be set or to be empty.
http://www.faqs.org/rfcs/rfc2183.html

This comment has been minimized.

Show comment
Hide comment
@oc666

oc666 Aug 23, 2012

@realityking, is this change apply to all 3 adapters ? (curl, socket, stream)

@realityking, is this change apply to all 3 adapters ? (curl, socket, stream)

This comment has been minimized.

Show comment
Hide comment
@realityking

realityking Aug 23, 2012

Maybe I'm overlooking a key thing here. I understood JHttpResponse::$transport to be an array of all the headers returned by the server. The keys being the header name and the value being the value.

As far as I can tell wrapper_data is something stream specific, if this is the case we should abstract this out in the JHttp transports.

@oc666 I'm not sure what you're asking, this code is outside JHttp, in theory it shouldn't concern itself with the question what transport is used.

Maybe I'm overlooking a key thing here. I understood JHttpResponse::$transport to be an array of all the headers returned by the server. The keys being the header name and the value being the value.

As far as I can tell wrapper_data is something stream specific, if this is the case we should abstract this out in the JHttp transports.

@oc666 I'm not sure what you're asking, this code is outside JHttp, in theory it shouldn't concern itself with the question what transport is used.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.