[GH-ISSUE #433] HTTP requests may be made to wrong URL #304

New issue

Open

opened 2026-02-28 01:24:04 +03:00 by kerem · 3 comments

kerem commented

2026-02-28 01:24:04 +03:00

Owner

Originally created by @alxndrsn on GitHub (Apr 8, 2016).
Original GitHub issue: https://github.com/ushahidi/SMSSync/issues/433

Both HTTP client implementations in the codebase, AppHttpClient and MessageHttpClient, depend on the same base class, BaseHttpClient.

Usage

`MessageHttpClient`

MessageHttpClient is mostly used in PostMessage. PostMessage is a singleton, and maintains a single reference to MessageHttpClient.

`AppHttpClient`

This class is mostly used by ProcessMessageResult, which is also a singleton and maintains a single reference to AppHttpClient.

The Problem

BaseHttpClient stores the URL to be contacted in a class property, which is done at various places:

This value is then read later in BaseHttpClient.execute() (in prepareRequest()).

In the intervening time between BaseHttpClient.mUrl being set in setUrl(), and being read in execute(), its value may have changed on another thread.

I had initially missed that both HTTP client classes are declared as singletons:

This presumably means that a single instance is shared wherever DI is used (which seems currently to cover all uses of these classes).

Originally created by @alxndrsn on GitHub (Apr 8, 2016). Original GitHub issue: https://github.com/ushahidi/SMSSync/issues/433 Both HTTP client implementations in the codebase, [`AppHttpClient`](https://github.com/ushahidi/SMSSync/blob/master/smssync/src/main/java/org/addhen/smssync/data/net/AppHttpClient.java) and [`MessageHttpClient`](https://github.com/ushahidi/SMSSync/blob/master/smssync/src/main/java/org/addhen/smssync/data/net/MessageHttpClient.java), depend on the same base class, [`BaseHttpClient`](https://github.com/ushahidi/SMSSync/blob/master/smssync/src/main/java/org/addhen/smssync/data/net/BaseHttpClient.java). # Usage ## `MessageHttpClient` `MessageHttpClient` is mostly used in `PostMessage`. [`PostMessage` is a singleton](https://github.com/ushahidi/SMSSync/blob/master/smssync/src/main/java/org/addhen/smssync/data/message/PostMessage.java#L56), and [maintains a single reference to `MessageHttpClient`](https://github.com/ushahidi/SMSSync/blob/master/smssync/src/main/java/org/addhen/smssync/data/message/PostMessage.java#L59). ## `AppHttpClient` This class is mostly used by [`ProcessMessageResult`](https://github.com/ushahidi/SMSSync/blob/master/smssync/src/main/java/org/addhen/smssync/data/message/ProcessMessageResult.java), which [is also a singleton](https://github.com/ushahidi/SMSSync/blob/master/smssync/src/main/java/org/addhen/smssync/data/message/ProcessMessageResult.java#L59) and maintains a single reference to `AppHttpClient`. # The Problem `BaseHttpClient` stores the URL to be contacted [in a class property](https://github.com/ushahidi/SMSSync/blob/master/smssync/src/main/java/org/addhen/smssync/data/net/BaseHttpClient.java#L70), which is done at various places: - https://github.com/ushahidi/SMSSync/blob/master/smssync/src/main/java/org/addhen/smssync/data/net/MessageHttpClient.java#L113 - https://github.com/ushahidi/SMSSync/blob/master/smssync/src/main/java/org/addhen/smssync/data/net/AppHttpClient.java#L70 - https://github.com/ushahidi/SMSSync/blob/master/smssync/src/main/java/org/addhen/smssync/data/net/BaseHttpClient.java#L70 This value is then read later in [`BaseHttpClient.execute()`](https://github.com/ushahidi/SMSSync/blob/master/smssync/src/main/java/org/addhen/smssync/data/net/BaseHttpClient.java#L70) (in `prepareRequest()`). In the intervening time between `BaseHttpClient.mUrl` being set in `setUrl()`, and being read in `execute()`, its value may have changed on another thread. --- I had initially missed that both HTTP client classes are declared as singletons: - https://github.com/ushahidi/SMSSync/blob/master/smssync/src/main/java/org/addhen/smssync/data/net/AppHttpClient.java#L50 - https://github.com/ushahidi/SMSSync/blob/master/smssync/src/main/java/org/addhen/smssync/data/net/MessageHttpClient.java#L53 This presumably means that a single instance is shared wherever DI is used (which seems currently to cover all uses of these classes).

kerem commented

2026-02-28 01:24:05 +03:00

Author

Owner

@eyedol commented on GitHub (Apr 22, 2016):

@alxndrsn I really don't understand this. Could you be clear on what needs changing or fixing? I need coffee :-)

@eyedol commented on GitHub (Apr 22, 2016): @alxndrsn I really don't understand this. Could you be clear on what needs changing or fixing? I need coffee :-)

kerem commented

2026-02-28 01:24:05 +03:00

Author

Owner

@alxndrsn commented on GitHub (Apr 22, 2016):

I think the simplest fix would be to use a fresh instance of MessageHttpClient/AppHttpClient for each request.

@alxndrsn commented on GitHub (Apr 22, 2016): I think the simplest fix would be to use a fresh instance of `MessageHttpClient`/`AppHttpClient` for each request.

kerem commented

2026-02-28 01:24:05 +03:00

Author

Owner

@eyedol commented on GitHub (Apr 27, 2016):

Got it

@eyedol commented on GitHub (Apr 27, 2016): Got it