frostwire / frostwire-jlibtorrent

A swig Java interface for libtorrent by the makers of FrostWire. Develop libtorrent based apps with the joy of coding in Java.
http://www.frostwire.com
MIT License
451 stars 138 forks source link

SessionManager alertsLoop crashes with null pointer exception if invalid Proxy is set #242

Closed vickyoo7 closed 3 years ago

vickyoo7 commented 3 years ago

When connection to proxy server is failed Alert type with 96 value is popped with message "SOCKS5 error. op: connect ec: Connection refused ep: 127.0.0.1:9050". Below code throws null pointer exception because AlertType Table doesn't have a value for index 96, there might be other values as well

alert a = v.get(i);
int type = a.type();
Alert<?> alert = null;
switch(AlertType.fromSwig(type)) {   //This is where null pointer exception is thrown
case SESSION_STATS:
    alert = Alerts.cast(a);
    SessionManager.this.stats.update((SessionStatsAlert)alert);
    break;
case PORTMAP:
    SessionManager.this.firewalled = false;
    break;
case PORTMAP_ERROR:
    SessionManager.this.firewalled = true;
    break;
case LISTEN_SUCCEEDED:
    alert = Alerts.cast(a);
    SessionManager.this.onListenSucceeded((ListenSucceededAlert)alert);
    break;
case LISTEN_FAILED:
    alert = Alerts.cast(a);
    SessionManager.this.onListenFailed((ListenFailedAlert)alert);
    break;
case EXTERNAL_IP:
    alert = Alerts.cast(a);
    SessionManager.this.onExternalIpAlert((ExternalIpAlert)alert);
    break;
case ADD_TORRENT:
    alert = Alerts.cast(a);
    if (SessionManager.this.isFetchMagnetDownload((AddTorrentAlert)alert)) {
        break label67;
    }
}
gubatron commented 3 years ago

Thank you for this feedback. Just added the new alert mapping in AlertType, it's called SOCKS5_ALERT.

Please try it with this jlibtorrent.jar (remove the .zip at the end of the filename, had to add it to drag into this comment), or git pull origin master and run from source.

jlibtorrent-1.2.10.0.jar.zip

vickyoo7 commented 3 years ago

Thanks, it works now but the possibility of similar crashes in future remains. Can you make the alertsloop thread more robust because if the thread crashes there's no way to handle it apart from catching it in UncaughtExceptionHandler, ignore it and stop the Session. Also I think returning UNKOWN for unmapped alert type is better

public static AlertType fromSwig(int swigValue) {
        return swigValue < 0 || swigValue >= TABLE.length ? UNKNOWN : TABLE[swigValue];
    }
gubatron commented 3 years ago

I believe the possibility of such cras in alertsloop is a feature, not a bug. If I remember @aldenml's coding style, these are things you don't want to fail silently, if an alert is missing we want to know it's missing.

vickyoo7 commented 3 years ago

Ok thanks, it makes sense