search

sozialhelden / wheelmap-classic

:wheelchair: Legacy "classic" wheelmap.org (deprecated)
http://www.wheelmap.org
GNU Affero General Public License v3.0
47 stars 16 forks source link

Wrong Results in Assets Api #145

Closed timfreiheit closed 8 years ago

timfreiheit commented 8 years ago

There is an critical bug in the assets api. The URLs in the json response are not valid.

http://wheelmap.org/api/assets?api_key=XXXXXXXX
{
  "conditions": {
    "page": 1,
    "per_page": 15,
    "format": "json",
    "locale": null
  },
  "meta": {
    "page": 1,
    "num_pages": 1,
    "item_count_total": 2,
    "item_count": 2
  },
  "assets": [
    {
      "id": 1,
      "name": "marker",
      "modified_at": 1455545130,
      "license": "cc-by-sa",
      "url": "//asset2.wheelmap.org/marker.zip?1455545130"
    },
    {
      "id": 2,
      "name": "icons",
      "modified_at": 1455545130,
      "license": "cc-by-sa",
      "url": "//asset3.wheelmap.org/icons.zip?1455545130"
    },
    {
      "id": 3,
      "name": "icons_white",
      "modified_at": 1455545130,
      "license": "cc-by-sa",
      "url": "//asset1.wheelmap.org/icons_white.zip?1455545130"
    }
  ]
}
lennerd commented 8 years ago

This is caused by the fix for #135, as we now use relative protocolls for asset urls to fix SSL issues in the webapp. @schultyy @1000miles Any ideas on how to make an exception for the API?

Here is the corresponding code snippet: https://github.com/sozialhelden/wheelmap/blob/master/app/models/asset.rb#L37

lennerd commented 8 years ago

Please don't get confused with the commit message, but be4c254340883761caba4446e7145a242fe8f4bf hopefully fixes the issue.

Had problems with an interactive rebase to squash multiple reverting commits into one which generated this confusing commit message.

holgerd commented 8 years ago

@timfreiheit please let me know if that fixed the issue. Then I will inform the Windows Phone user who already emailed us reporting this problem.

timfreiheit commented 8 years ago

@holgerd it is fixed :)

timfreiheit commented 8 years ago

@holgerd @lennerd The bug occurs again. I'm not sure if I should create a new issue for this.

lennerd commented 8 years ago

I will take a look into this.

@timfreiheit I thought the Zip files are not used anymore?

timfreiheit commented 8 years ago

they are on all platforms. At least the assets with the name "icons". Is there any replacement?

lennerd commented 8 years ago

Oh no. This was a total misunderstanding between me and @holgerd. Using the icon zip files was added to all apps a few months ago, right?

My plan was originally to change all apps to use the node_type API and the icons named there. I will discuss this with @holgerd.

lennerd commented 8 years ago

Example: http://wheelmap.org/api/categories/1/node_types?api_key=XXX

{
   "conditions":{
      "page":1,
      "per_page":200,
      "format":"json",
      "locale":"de"
   },
   "meta":{
      "page":1,
      "num_pages":1,
      "item_count_total":16,
      "item_count":16
   },
   "node_types":[
      {
         "id":1,
         "identifier":"bicycle_rental",
         "category_id":1,
         "category":{
            "id":1,
            "identifier":"public_transfer"
         },
         "localized_name":"Fahrradvermietung",
         "icon":"cycling.png"
      },
      {
         "id":2,
         "identifier":"boatyard",
         "category_id":1,
         "category":{
            "id":1,
            "identifier":"public_transfer"
         },
         "localized_name":"Hafen",
         "icon":"boat.png"
      },
      {
         "id":3,
         "identifier":"bus_station",
         "category_id":1,
         "category":{
            "id":1,
            "identifier":"public_transfer"
         },
         "localized_name":"Busbahnhof",
         "icon":"busstop.png"
      },
      {
         "id":4,
         "identifier":"bus_stop",
         "category_id":1,
         "category":{
            "id":1,
            "identifier":"public_transfer"
         },
         "localized_name":"Bushaltestelle",
         "icon":"busstop.png"
      },
      {
         "id":5,
         "identifier":"car_rental",
         "category_id":1,
         "category":{
            "id":1,
            "identifier":"public_transfer"
         },
         "localized_name":"Autovermietung",
         "icon":"carrental.png"
      },
      {
         "id":6,
         "identifier":"car_sharing",
         "category_id":1,
         "category":{
            "id":1,
            "identifier":"public_transfer"
         },
         "localized_name":"Carsharing",
         "icon":"carrental.png"
      },
      {
         "id":7,
         "identifier":"ferry_terminal",
         "category_id":1,
         "category":{
            "id":1,
            "identifier":"public_transfer"
         },
         "localized_name":"F\u00e4hranlegestelle",
         "icon":"ferry.png"
      },
      {
         "id":8,
         "identifier":"fuel",
         "category_id":1,
         "category":{
            "id":1,
            "identifier":"public_transfer"
         },
         "localized_name":"Tankstelle",
         "icon":"fillingstation.png"
      },
      {
         "id":9,
         "identifier":"halt",
         "category_id":1,
         "category":{
            "id":1,
            "identifier":"public_transfer"
         },
         "localized_name":"Haltestelle",
         "icon":"train.png"
      },
      {
         "id":10,
         "identifier":"parking",
         "category_id":1,
         "category":{
            "id":1,
            "identifier":"public_transfer"
         },
         "localized_name":"Parkplatz/Parkhaus",
         "icon":"parking.png"
      },
      {
         "id":11,
         "identifier":"platform",
         "category_id":1,
         "category":{
            "id":1,
            "identifier":"public_transfer"
         },
         "localized_name":"Bahnsteig",
         "icon":"train.png"
      },
      {
         "id":12,
         "identifier":"station",
         "category_id":1,
         "category":{
            "id":1,
            "identifier":"public_transfer"
         },
         "localized_name":"Bahnhof",
         "icon":"train.png"
      },
      {
         "id":13,
         "identifier":"subway_entrance",
         "category_id":1,
         "category":{
            "id":1,
            "identifier":"public_transfer"
         },
         "localized_name":"U-Bahn Eingang",
         "icon":"underground.png"
      },
      {
         "id":14,
         "identifier":"terminal",
         "category_id":1,
         "category":{
            "id":1,
            "identifier":"public_transfer"
         },
         "localized_name":"Flughafenterminal",
         "icon":"airport_terminal.png"
      },
      {
         "id":15,
         "identifier":"tram_stop",
         "category_id":1,
         "category":{
            "id":1,
            "identifier":"public_transfer"
         },
         "localized_name":"Stra\u00dfenbahnhaltestelle",
         "icon":"tramway.png"
      },
      {
         "id":16,
         "identifier":"cable_car",
         "category_id":1,
         "category":{
            "id":1,
            "identifier":"public_transfer"
         },
         "localized_name":"Seilbahnstation",
         "icon":"cablecar.png"
      }
   ]
}
timfreiheit commented 8 years ago

this is how it is implemented already.

lennerd commented 8 years ago

Sorry, what exactly is implemented? Loading the icons via ZIP file or seperatly via the NodeType API?

timfreiheit commented 8 years ago

both. the apps use the /api/node_types api to create the association between the images from the assets and the specific node by its type.

lennerd commented 8 years ago

@timfreiheit Does #206 fix this?

timfreiheit commented 8 years ago

the api issue is fixed. But i'm getting an ssl exception when trying to load the zip on staging.

javax.net.ssl.SSLHandshakeException: java.security.cert.CertPathValidatorException: Trust anchor for certification path not found.
lennerd commented 8 years ago

@Xylakant Any ideas what this could mean?

lennerd commented 8 years ago

@timfreiheit Does this help? http://stackoverflow.com/questions/2642777/trusting-all-certificates-using-httpclient-over-https/6378872#6378872

Xylakant commented 8 years ago

Either an intermediate cert is missing or staging still uses the self-signed cert. I'll check tomorrow.

Sent from my iPhone

On 26.04.2016, at 21:51, Lennart Hildebrandt notifications@github.com wrote:

@Xylakant Any ideas what this could mean?

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub

timfreiheit commented 8 years ago

there is a problem with the ssl certification. Of cause the app could just trust it. But this is a bad/error-prone solution and apps using insecure implementations of the X509TrustManager would be rejected by the google.

lennerd commented 8 years ago

@timfreiheit Did you test against staging or production server?

timfreiheit commented 8 years ago

the problem appears only on staging. the production endpoint is good

lennerd commented 8 years ago

@timfreiheit Can you please create an issue for this and assign Xylakant to it?

Xylakant commented 8 years ago

Trusting a specific (self signed) cert is certainly an acceptable solution that - if implemented correctly - is more secure than trusting a list of CAs. However, it's significantly more management overhead and total overkill in our case.

Sent from my iPhone

On 26.04.2016, at 21:58, Tim Freiheit notifications@github.com wrote:

there is a problem with the ssl certification. Of cause the app could just trust it. But this is a bad/error-prone solution and apps using insecure implementations of the X509TrustManager would be rejected by the google.

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub