search

unfoldingWord-dev / uw-android

Android unfoldingWord app
Other
9 stars 4 forks source link

Zechariah Issue #88

Closed jag3773 closed 9 years ago

jag3773 commented 9 years ago

Zechariah is failing to validate, but it does validate on the server, so I think the signature is good.

bknatterud commented 9 years ago

In the JSON the sit URL for Zechariah is 404

https://api.unfoldingword.org/ulb/txt/1/ulb-en/38-ZEC.sig

jag3773 commented 9 years ago

OK, the app needs to skip 404s.

bknatterud commented 9 years ago

I can skip them but then what should be presented to the user. Technically the content is not verified at that point so it does not seem like we should show a green check. What should the user interface do differently if a link is 404?

jag3773 commented 9 years ago

There is no content for zec as far as I can tell. That's what I mean by skipping 404 content requests.

bknatterud commented 9 years ago

I think that it may be better to not include it in the JSON if there is no content for it. Is there a reason why a book of the Bible with no content needs to be included in the JSON?

jag3773 commented 9 years ago

I agree, but stranger things will happen. The app should be smart enough to not try to verify something that doesn't exist.

bknatterud commented 9 years ago

I think one unintended consequence of doing that is that if things like that are going to happen on the server and for some reason that issue can't be handled on the server isn't it good that the app is able to bring to our attention a server issue that is occurring? If the app just goes along and verifies everything and skips something that is 404 it could take sometime before somebody notices that a book is not coming into the app. I know one thing we usually like to try and do is handle as much as possible on the server since that can be updated much faster then a mobile app. Just some thoughts.

jag3773 commented 9 years ago

User experience trumps all. User's are not our beta testers. The app should transparently do the right thing.

Throwing a big warning that the content is not verified is a great way to strike fear into users and encourage them to never use the app :(.

Automated testing should identify server side problems.

bknatterud commented 9 years ago

Yes. Definitely not good user experience to see a red icon. But I think the issue is a server issue that should be addressed.

I was just talking with the team here and these are our main concerns in making this change.

  1. this potentially opens up security holes to exploit verification
  2. apps can't take random server failures and just automatically handle them. I think it is reasonable to expect the server to give valid information
  3. if we can't rely on the server to give valid information what are the potential other cases in the future the app will be expected to handle?

Ultimately yes, we can handle this like you say, but it feels like we are addressing the issue in the wrong place when it should be addressed on the server.

If it is imperative we handle in the app we can, but want you to know that we have reservations about it.

jag3773 commented 9 years ago
  1. Examples? A 404 on the content means that there is no content, how can we have an exploit on a null-op? A verified nothing? A not verified anything?
  2. We are not talking about random server errors but a very specific 404 error.
  3. You cannot rely on the server, there will very likely be future cases like this that we'll run into.
bknatterud commented 9 years ago

finishing this up in the next hour or two