Matthias247 / jawampa

Web Application Messaging Protocol (WAMP v2) support for Java
Apache License 2.0
148 stars 57 forks source link

Invalid URI exception (on pretty valid URI) #30

Closed ghost closed 9 years ago

ghost commented 9 years ago

I'm getting "invalid_uri" error when trying to subscribe to feed "BTC_XMR". Here is the code to reproduce.

public static void main(String[] args) {
    try {
      WampClient client = new WampClientBuilder()
          .withUri("wss://api.poloniex.com")
          .withRealm("realm1")
          .build();

      // open connection
      client.open();

      // subscribe to connection status events
      client.statusChanged()
          .observeOn(Schedulers.newThread())
          .subscribe((WampClient.Status status) -> {
            switch (status) {
              case Connected:
                System.out.println("INFO - CONNECTED");

                // perform subscriptions
                client
                    .makeSubscription("BTC_XMR")
                    .observeOn(Schedulers.newThread())
                    .subscribe(
                        (x) -> {
                          System.out.println(toPrettyJSON(x));
                        },
                        Throwable::printStackTrace
                    );
                break;
              case Disconnected:
                System.out.println("INFO - DISCONNECTED");
                break;
              case Connecting:
                System.out.println("INFO - CONNECTING");
                break;
              default:
                throw new IllegalStateException("unknown status");
            }
          });

      bearSleep(50000);

      client.close();
    } catch (WampError e) {
      e.printStackTrace();
    }
  }

bearSleep and toPrettyJson can be replaced with something more simple. exception i get for this is

ApplicationError(wamp.error.invalid_uri, [], {})
    at ws.wamp.jawampa.internal.UriValidator.validate(UriValidator.java:77)

Please fix this, i'm blocked because of this issue...

ghost commented 9 years ago

I've forked and commented out that part of the code that does validation on makeSubscription, not sure why you're validating that for uri at all? I believe some sort of validation is ok but clearly "BTC_XMT" is pretty valid feed to make subscription.

Probably something is wrong with regex or you need a different regex for feeds... (i'd left only not null and not empty check)

Matthias247 commented 9 years ago

Hi, this is because jawampa uses the strict URI validation rules with the corresponding regexes that are described and recommended in the WAMP specification: https://github.com/tavendo/WAMP/blob/master/spec/basic.md#identifiers . These don't allow uppercase characters. btc_xmt would be a valid URI.

The issue is a duplicate of #11 and #12

You are right that there's no absolute need to validate the URIs at client side because it's also done at server side. However it doesn't cost much, can avoid unnecessary messages round trips and leads to consistent results even with buggy or mocked server implementations. So I thought it makes sense to have it.

ghost commented 9 years ago

Lets thing from different perspective, the api provider in my calse poloniex didnt take into consideration that recomended way of defining feeds, and it defined that i must subscribed to uppercase version, now that i'm a client and using your lib i have no way to use their api (altough yes its their falt not following the guidline but they dont really have to) except what i just did, i must fork your project comment out some logic in order to use a lib? i dont think thats a reasonable.

At least i must have option to choose if i want it to be validated, from my perspective i dont care what they define, i care that my data gets to me....

So my suggestion is to remove that check from there... Its harming us clients not the providers. I'll write them about that tough... will link to this issue.

ghost commented 9 years ago

My point is that you dont want to enforce that check in your lib, now i'm worked from your project, unless you fix this or poloniex fix their server side (which i seriously doubt because they have existing clients) i wont be able to get back to remote maven dependency, and will not get future versions (if not done mannually)

santhosh-tekuri commented 9 years ago
WampClientBuilder.withStrictUriValidation(false)

this can be used to relax validation at client side

ghost commented 9 years ago

How do we track if these changes are in latest maven build? in maven repo... Does it change version? or i have to build it manually?

Matthias247 commented 9 years ago

Hi, this is available since a change I made at the weekend. And also the default is now loose validation - so for your case it's even not necessary to set the parameter.

The change is not yet in the newest maven build - that's why I didn't mentioned it here yet. It will be in the next one - but I wanted to look at some other things first and want to avoid making new release versions all the time.

ghost commented 9 years ago

Ok, this might be not related question but once it versioned and available trough maven repo, can it change but keep same version? or you must change the version? i dont know how its done usually...

I suppose that to see this update in maven i must check for newer version...

Matthias247 commented 9 years ago

If a new version gets available it will also have a new (increased) version number. Otherwise it could break the build for all people which are having a dependency on the current version.