Merge pull request #124 from proximax-storage/issue/122-method_not_allowed

Transaction announcement does not work after security audit

Author: tonowie

CHANGELOG.md

@@ -1,5 +1,10 @@
 # CHANGELOG
 
+## 0.5.1 Security audit
+
+Fixing issues caused by changes introduced because of security audit
+* remove trailing slash from resource URLs
+
 ## 0.5.0 MaxFee calculation and transaction builders
 
 See [milestone](https://github.com/proximax-storage/java-xpx-chain-sdk/milestone/5?closed=1) for fixed issues

gradle.properties

@@ -1,4 +1,4 @@
 # repository group
 group=io.proximax
 # current version
-version=0.5.0-SNAPSHOT
+version=0.5.1-SNAPSHOT

src/e2e/java/io/proximax/sdk/E2EBaseTest.java

@@ -56,7 +56,7 @@ public class E2EBaseTest extends BaseTest {
    /** logger */
    private static final Logger logger = LoggerFactory.getLogger(E2EBaseTest.class);
 
-   protected static final BigInteger DEFAULT_DEADLINE_DURATION = BigInteger.valueOf(5*60*1000l);
+   protected static final BigInteger DEFAULT_DEADLINE_DURATION = BigInteger.valueOf(60*60*1000l);
 
    protected BlockchainApi api;
    protected BlockchainRepository blockchainHttp;
@@ -187,7 +187,7 @@ protected void returnAllToSeed(Account from) {
             sendMosaic(from, seedAccount.getAddress(), mosaic);
          });
       } catch (RuntimeException e) {
-         if (!"Not Found".equals(e.getMessage())) {
+         if (!"404 Not Found".equals(e.getMessage())) {
             fail(e);
          }
       }

src/main/java/io/proximax/sdk/infrastructure/AccountHttp.java

@@ -51,7 +51,7 @@
  */
 public class AccountHttp extends Http implements AccountRepository {
 
-   private static final String ROUTE = "/account/";
+   private static final String ROUTE = "/account";
    private static final String PROPERTIES_SUFFIX = "/properties";
 
    private static final Type TYPE_ACCOUNT_LIST = new TypeToken<List<AccountInfoDTO>>(){}.getType();
@@ -64,7 +64,7 @@ public AccountHttp(BlockchainApi api) {
 
    @Override
    public Observable<AccountInfo> getAccountInfo(Address address) {
-      return this.client.get(ROUTE + address.plain()).map(Http::mapStringOrError)
+      return this.client.get(ROUTE + SLASH + address.plain()).map(Http::mapStringOrError)
             .map(str -> gson.fromJson(str, AccountInfoDTO.class))
             .map(AccountInfo::fromDto);
    }
@@ -99,22 +99,22 @@ public Observable<List<AccountInfo>> getAccountsInfo(List<Address> addresses) {
 
    @Override
    public Observable<MultisigAccountInfo> getMultisigAccountInfo(Address address) {
-      return this.client.get(ROUTE + address.plain() + "/multisig").map(Http::mapStringOrError)
+      return this.client.get(ROUTE + SLASH + address.plain() + "/multisig").map(Http::mapStringOrError)
             .map(str -> gson.fromJson(str, MultisigAccountInfoDTO.class))
             .map(dto -> MultisigAccountInfo.fromDto(dto, api.getNetworkType()));
    }
 
    @Override
    public Observable<MultisigAccountGraphInfo> getMultisigAccountGraphInfo(Address address) {
-      return this.client.get(ROUTE + address.plain() + "/multisig/graph")
+      return this.client.get(ROUTE + SLASH + address.plain() + "/multisig/graph")
             .map(Http::mapStringOrError)
             .map(this::toMultisigAccountInfo)
             .map(dto -> MultisigAccountGraphInfo.fromDto(dto, api.getNetworkType()));
    }
 
    @Override
    public Observable<AccountProperties> getAccountProperties(Address address) {
-      return this.client.get(ROUTE + address.plain() + PROPERTIES_SUFFIX).map(Http::mapStringOrError)
+      return this.client.get(ROUTE + SLASH + address.plain() + PROPERTIES_SUFFIX).map(Http::mapStringOrError)
             .map(str -> gson.fromJson(str, AccountPropertiesInfoDTO.class))
             .map(AccountPropertiesInfoDTO::getAccountProperties).map(AccountProperties::fromDto);
    }
@@ -224,8 +224,7 @@ private Observable<List<Transaction>> unconfirmedTransactions(PublicAccount publ
    private Observable<List<Transaction>> findTransactions(String accountKey,
          Optional<QueryParams> queryParams, String path) {
       return this.client
-            .get(ROUTE
-                  + accountKey + path + (queryParams.isPresent() ? queryParams.get().toUrl() : ""))
+            .get(ROUTE + SLASH + accountKey + path + (queryParams.isPresent() ? queryParams.get().toUrl() : ""))
             .map(Http::mapStringOrError)
             .map(str -> stream(new Gson().fromJson(str, JsonArray.class)).map(s -> (JsonObject) s)
                   .collect(Collectors.toList()))

src/main/java/io/proximax/sdk/infrastructure/BlockchainHttp.java

@@ -47,8 +47,8 @@ public class BlockchainHttp extends Http implements BlockchainRepository {
    private static final String BLOCK = "/block/";
    private static final String CHAIN_HEIGHT = "/chain/height";
    private static final String CHAIN_SCORE = "/chain/score";
-   private static final String CONFIG = "/config/";
-   private static final String UPGRADE = "/upgrade/";
+   private static final String CONFIG = "/config";
+   private static final String UPGRADE = "/upgrade";
 
    private static final Type BLOCK_INFO_LIST_TYPE = new TypeToken<List<BlockInfoDTO>>(){}.getType();
 
@@ -194,7 +194,7 @@ public Observable<List<BlockInfo>> getBlocksByHeightWithLimit(BigInteger height,
 
    @Override
    public Observable<BlockchainConfig> getBlockchainConfiguration(BigInteger height) {
-      return this.client.get(CONFIG + height.toString())
+      return this.client.get(CONFIG + SLASH + height.toString())
             .map(Http::mapStringOrError)
             .map(str -> gson.fromJson(str, CatapultConfigDTO.class))
             .map(CatapultConfigDTO::getCatapultConfig)
@@ -203,7 +203,7 @@ public Observable<BlockchainConfig> getBlockchainConfiguration(BigInteger height
 
    @Override
    public Observable<BlockchainUpgrade> getBlockchainUpgrade(BigInteger height) {
-      return this.client.get(UPGRADE + height.toString())
+      return this.client.get(UPGRADE + SLASH + height.toString())
             .map(Http::mapStringOrError)
             .map(str -> gson.fromJson(str, CatapultUpgradeDTO.class))
             .map(CatapultUpgradeDTO::getCatapultConfig)

src/main/java/io/proximax/sdk/infrastructure/ContractHttp.java

@@ -25,9 +25,9 @@
  */
 public class ContractHttp extends Http implements ContractRepository {
 
-   private static final String CONTRACT_ROUTE = "/contract/";
+   private static final String CONTRACT_ROUTE = "/contract";
    private static final String CONTRACS_SUFFIX = "/contracts";
-   private static final String ACCOUNT_ROUTE = "/account/";
+   private static final String ACCOUNT_ROUTE = "/account";
    private static final String ACCOUNT_CONTRACTS_ROUTE = "/account/contracts";
 
    private static final Type CONTRACT_INFO_LIST_TYPE = new TypeToken<List<ContractInfoDTO>>(){}.getType();
@@ -38,7 +38,7 @@ public ContractHttp(BlockchainApi api) {
 
    @Override
    public Observable<Contract> getContract(Address address) {
-      return this.client.get(CONTRACT_ROUTE + address.plain())
+      return this.client.get(CONTRACT_ROUTE + SLASH + address.plain())
             .map(Http::mapStringOrError)
             .map(str -> gson.fromJson(str, ContractInfoDTO.class))
             .map(ContractInfoDTO::getContract)
@@ -64,7 +64,7 @@ public Observable<List<Contract>> getContracts(Address... addresses) {
 
    @Override
    public Observable<Contract> getContract(PublicKey publicKey) {
-      return this.client.get(ACCOUNT_ROUTE + publicKey.getHexString() + CONTRACS_SUFFIX)
+      return this.client.get(ACCOUNT_ROUTE + SLASH + publicKey.getHexString() + CONTRACS_SUFFIX)
             .map(Http::mapStringOrError)
             .map(this::toContractInfoList)
             .flatMapIterable(item -> item)

src/main/java/io/proximax/sdk/infrastructure/Http.java

@@ -28,6 +28,8 @@
  * base HTTP repository implementation, keeping track of the API, HTTP client and mapper
  */
 public class Http {
+   protected static final String SLASH = "/";
+   
    protected final BlockchainApi api;
    protected final HttpClient client;
    protected final Gson gson;
@@ -74,7 +76,7 @@ public Gson getGson() {
     */
    static String mapStringOrError(final HttpResponse response) {
       if (response.getCode() < 200 || response.getCode() > 299) {
-         throw new RuntimeException(response.getStatusMessage());
+         throw new RuntimeException(response.getCode() + " " + response.getStatusMessage());
       }
       try {
          return response.getBodyString();

src/main/java/io/proximax/sdk/infrastructure/MetadataHttp.java

@@ -41,7 +41,7 @@
  * Metadata http repository.
  */
 public class MetadataHttp extends Http implements MetadataRepository {
-   private static final String URL_METADATA = "/metadata/";
+   private static final String URL_METADATA = "/metadata";
    private static final String URL_ACCOUNT = "/account/";
    private static final String URL_MOSAIC = "/mosaic/";
    private static final String URL_NAMESPACE = "/namespace/";
@@ -55,7 +55,7 @@ public MetadataHttp(BlockchainApi api) {
    @Override
    public Observable<Metadata> getMetadata(String metadataId) {
       return this.client
-         .get(URL_METADATA + metadataId)
+         .get(URL_METADATA + SLASH + metadataId)
          .map(Http::mapStringOrError)
          .map(GsonUtils::mapToJsonObject)
          .map(MetadataMapper::mapToObject);

src/main/java/io/proximax/sdk/infrastructure/MosaicHttp.java

@@ -41,7 +41,7 @@
  */
 public class MosaicHttp extends Http implements MosaicRepository {
 
-   private static final String ROUTE = "/mosaic/";
+   private static final String ROUTE = "/mosaic";
    private static final String NAMES_ROUTE = "/mosaic/names";
 
    private static final Type MOSAIC_INFO_LIST_TYPE = new TypeToken<List<MosaicInfoDTO>>(){}.getType();
@@ -53,7 +53,7 @@ public MosaicHttp(BlockchainApi api) {
 
    @Override
    public Observable<MosaicInfo> getMosaic(MosaicId mosaicId) {
-      return this.client.get(ROUTE + mosaicId.getIdAsHex())
+      return this.client.get(ROUTE + SLASH + mosaicId.getIdAsHex())
             .map(Http::mapStringOrError)
             .map(str -> gson.fromJson(str, MosaicInfoDTO.class))
             .map(dto -> MosaicInfo.fromDto(dto, api.getNetworkType()));

src/main/java/io/proximax/sdk/infrastructure/TransactionHttp.java

@@ -46,7 +46,7 @@
  * @since 1.0
  */
 public class TransactionHttp extends Http implements TransactionRepository {
-	private static final String ROUTE = "/transaction/";
+   private static final String ROUTE = "/transaction";
    private static final String KEY_MESSAGE = "message";
    private static final String KEY_PAYLOAD = "payload";
 
@@ -59,7 +59,7 @@ public TransactionHttp(BlockchainApi api) {
     @Override
     public Observable<Transaction> getTransaction(String transactionHash) {
         return this.client
-                .get(ROUTE + transactionHash)
+                .get(ROUTE + SLASH + transactionHash)
                 .map(Http::mapStringOrError)
                 .map(GsonUtils::mapToJsonObject)
                 .map(new TransactionMapping());
@@ -85,7 +85,7 @@ public Observable<List<Transaction>> getTransactions(List<String> transactionHas
     @Override
     public Observable<TransactionStatus> getTransactionStatus(String transactionHash) {
         return this.client
-                .get(ROUTE + transactionHash + "/status")
+                .get(ROUTE + SLASH + transactionHash + "/status")
                 .map(Http::mapStringOrError)
                 .map(str -> gson.fromJson(str, TransactionStatusDTO.class))
                 .map(transactionStatusDTO -> new TransactionStatus(transactionStatusDTO.getGroup(),