1208587 - SUTAgent is using Apache HttpClient library that has been removed in Android API Level 23

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Testing, defect)

Description

[Sebastian Kaspari (:sebastian; :pocmo)]

SUTAgent uses org.apache.http.*. This package has been removed in Android API level 23 (M / 6.0).

Build errors:
https://pastebin.mozilla.org/8847539

Fennec switched from org.apache to ch.boye in bug 1169421.

Comment 1

[Sebastian Kaspari (:sebastian; :pocmo)]

(In reply to Sebastian Kaspari (:sebastian) from comment #0)
> Fennec switched from org.apache to ch.boye in bug 1169421.

Another option would be to switch to URLConnection to avoid adding a third-party dependency. I guess that depends on whether SUTAgent is just doing simple requests or uses some fancy HTTP features.

From the build log it looks like SUTAgent calls Notification.setLatestEventInfo() that is deprecated now. In Fennec we switched to using NotificationCompat.Builder (Bug 1195692).

Comment 2

[Sebastian Kaspari (:sebastian; :pocmo)]

gbrown, jmaher: What do you think would be the best way to go forward here?

Comment 3

[Joel Maher ( :jmaher ) (UTC -8)]

I suspect we might be able to get rid of the funciton GetInternetData:
https://dxr.mozilla.org/mozilla-central/search?q=GetInternetData&case=true

To be honest I am not sure if there is anyway to use the GetInternetData in other non obvious ways.  Most likely it isn't used at all and given the fact that nothing calls it there is high confidence we don't need it anytmore.

If you remove that entire function and the related imports, will the build work?

Comment 4

[Sebastian Kaspari (:sebastian; :pocmo)]

Thanks! I'm going to try that (and refactor the Notification.setLatestEventInfo usage).

Comment 5

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: SUTAgent: Remove unused method GetInternetData()

Comment 6

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: SUTAgent: Replace deprecated Notification APIs with Notification.Builder

Comment 7

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: SUTAgent: Inline isIPv4Address()

Comment 8

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8668467 [details] [diff] [review]
SUTAgent: Remove unused method GetInternetData()

Review of attachment 8668467 [details] [diff] [review]:
-----------------------------------------------------------------

great!

Comment 9

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8668469 [details] [diff] [review]
SUTAgent: Inline isIPv4Address()

Review of attachment 8668469 [details] [diff] [review]:
-----------------------------------------------------------------

other than my confusion on the regex, this is good

::: build/mobile/sutagent/android/SUTAgentAndroid.java
@@ +55,5 @@
>  public class SUTAgentAndroid extends Activity
>      {
>      final Handler mHandler = new Handler();
>  
> +    private static final Pattern IPV4_PATTERN = Pattern.compile("^(25[0-5]|2[0-4]\\d|[0-1]?\\d?\\d)(\\.(25[0-5]|2[0-4]\\d|[0-1]?\\d?\\d)){3}$");

I don't understand this pattern?  how does loopback work or other addresses work?  is there a reference for this?

Comment 10

[Sebastian Kaspari (:sebastian; :pocmo)]

(In reply to Joel Maher (:jmaher) from comment #9)
> Comment on attachment 8668469 [details] [diff] [review]
> SUTAgent: Inline isIPv4Address()
> 
> Review of attachment 8668469 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> other than my confusion on the regex, this is good
> 
> ::: build/mobile/sutagent/android/SUTAgentAndroid.java
> @@ +55,5 @@
> >  public class SUTAgentAndroid extends Activity
> >      {
> >      final Handler mHandler = new Handler();
> >  
> > +    private static final Pattern IPV4_PATTERN = Pattern.compile("^(25[0-5]|2[0-4]\\d|[0-1]?\\d?\\d)(\\.(25[0-5]|2[0-4]\\d|[0-1]?\\d?\\d)){3}$");
> 
> I don't understand this pattern?  how does loopback work or other addresses
> work?  is there a reference for this?

I basically just inlined the code from InetAddressUtils:
http://androidxref.com/source/xref/external/apache-http/src/org/apache/http/conn/util/InetAddressUtils.java

I just saw that the latest HttpClient release has a refactored, more readabable, version:

> private static final String IPV4_BASIC_PATTERN_STRING =
>             "(([1-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\\.){1}" + // initial first field, 1-255
>             "(([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\\.){2}" + // following 2 fields, 0-255 followed by .
>              "([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])"; // final field, 0-255

http://grepcode.com/file/repo1.maven.org/maven2/org.apache.httpcomponents/httpclient/4.5/org/apache/http/conn/util/InetAddressUtils.java#45

Comment 11

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8668469 [details] [diff] [review]
SUTAgent: Inline isIPv4Address()

Review of attachment 8668469 [details] [diff] [review]:
-----------------------------------------------------------------

given that this is the same regex as used in the code we were importing, lets r+.  Please reference where you did get it from in a comment.

Comment 12

[Sebastian Kaspari (:sebastian; :pocmo)]

Together with other API 23 build patches:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=93b51c8b5499

Comment 13

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8668468 [details] [diff] [review]
SUTAgent: Replace deprecated Notification APIs with Notification.Builder

Mike has so much in his review queue, maybe you have less? :)

Comment 14

[Sebastian Kaspari (:sebastian; :pocmo)]

leave-open: I'm going to land the two reviewed patches.

Comment 15

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/488526a93c27
https://hg.mozilla.org/integration/fx-team/rev/43e9af141078

Comment 16

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/488526a93c27
https://hg.mozilla.org/mozilla-central/rev/43e9af141078

Comment 17

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8668468 [details] [diff] [review]
SUTAgent: Replace deprecated Notification APIs with Notification.Builder

Review of attachment 8668468 [details] [diff] [review]:
-----------------------------------------------------------------

Seems reasonable to me.

Comment 18

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/df5a6ba88fed

Comment 19

[Geoff Brown [:gbrown]]

Sorry to be so late to the party! Patches look good to me.

Comment 20

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/df5a6ba88fed