From e86426f4f2428e7ddec4645ecc58574c2c4c81d7 Mon Sep 17 00:00:00 2001
From: uhensler <none@none>
Date: Tue, 31 Oct 2017 12:45:12 +0100
Subject: [PATCH] OO-2916: Better exception handling of unavailable cover
 images, more validations of page from/to, utf-8 of infodoc response

---
 .../EdubaseBookSectionListController.java     | 36 ++++++++++++++-----
 .../edubase/_content/bookSectionList.html     |  6 ++--
 .../overview_description_disabled.html        |  4 +--
 .../overview_description_enabled.html         |  4 +--
 .../nodes/edubase/_content/peekview.html      |  4 +--
 .../edubase/_i18n/LocalStrings_de.properties  |  1 +
 .../edubase/_i18n/LocalStrings_en.properties  |  1 +
 .../edubase/manager/EdubaseManagerImpl.java   | 33 ++---------------
 8 files changed, 38 insertions(+), 51 deletions(-)

diff --git a/src/main/java/org/olat/course/nodes/edubase/EdubaseBookSectionListController.java b/src/main/java/org/olat/course/nodes/edubase/EdubaseBookSectionListController.java
index ec88e0c5454..61c342047c2 100644
--- a/src/main/java/org/olat/course/nodes/edubase/EdubaseBookSectionListController.java
+++ b/src/main/java/org/olat/course/nodes/edubase/EdubaseBookSectionListController.java
@@ -37,6 +37,7 @@ import org.olat.core.gui.control.Controller;
 import org.olat.core.gui.control.Event;
 import org.olat.core.gui.control.WindowControl;
 import org.olat.core.gui.control.generic.closablewrapper.CloseableModalController;
+import org.olat.core.logging.AssertException;
 import org.olat.core.util.StringHelper;
 import org.olat.course.nodes.EdubaseCourseNode;
 import org.olat.modules.ModuleConfiguration;
@@ -138,21 +139,23 @@ public class EdubaseBookSectionListController extends FormBasicController {
 					allOk = false;
 				}
 			}
-			allOk &= validateInt(bookSectionWrapper.getPageFromEl());
-			allOk &= validateInt(bookSectionWrapper.getPageToEl());
+			allOk &= validatePositiveInt(bookSectionWrapper.getPageFromEl());
+			allOk &= validatePositiveInt(bookSectionWrapper.getPageToEl());
+			allOk &= validateToHigherFrom(bookSectionWrapper.getPageFromEl(), bookSectionWrapper.getPageToEl());
 		}
 
 		return allOk & super.validateFormLogic(ureq);
 	}
 
-	private boolean validateInt(TextElement el) {
+	private boolean validatePositiveInt(TextElement el) {
 		boolean allOk = true;
 
 		if(el.isVisible()) {
 			String value = el.getValue();
 			if(StringHelper.containsNonWhitespace(value)) {
 				try {
-					Integer.parseInt(value);
+					Integer intValue = Integer.parseInt(value);
+					if (intValue <= 0) throw new AssertException("negativ number");
 				} catch(Exception e) {
 					el.setErrorKey("form.error.wrong.int", null);
 					allOk = false;
@@ -162,6 +165,23 @@ public class EdubaseBookSectionListController extends FormBasicController {
 
 		return allOk;
 	}
+	
+	private boolean validateToHigherFrom(TextElement fromEl, TextElement toEl) {
+		boolean allOk = true;
+		
+		try {
+			Integer from = Integer.parseInt(fromEl.getValue());
+			Integer to = Integer.parseInt(toEl.getValue());
+			if (from > 0 && to > 0 && from > to) {
+				toEl.setErrorKey("form.error.page.to.higher.from", null);
+				allOk = false;
+			}
+		} catch (Exception e) {
+			// validate only if integers
+		}
+		
+		return allOk;
+	}
 
 	@Override
 	protected void formInnerEvent(UserRequest ureq, FormItem source, FormEvent event) {
@@ -443,14 +463,14 @@ public class EdubaseBookSectionListController extends FormBasicController {
 
 		public BookSection getBookSection() {
 			bookSection.setBookId(bookIdEl.getValue());
-			if (StringHelper.containsNonWhitespace(pageFromEl.getValue())) {
+			try {
 				bookSection.setPageFrom(Integer.valueOf(pageFromEl.getValue()));
-			} else {
+			} catch (Exception e) {
 				bookSection.setPageFrom(null);
 			}
-			if (StringHelper.containsNonWhitespace(pageToEl.getValue())) {
+			try {
 				bookSection.setPageTo(Integer.valueOf(pageToEl.getValue()));
-			} else {
+			} catch (Exception e) {
 				bookSection.setPageTo(null);
 			}
 			if (StringHelper.containsNonWhitespace(titleEl.getValue())) {
diff --git a/src/main/java/org/olat/course/nodes/edubase/_content/bookSectionList.html b/src/main/java/org/olat/course/nodes/edubase/_content/bookSectionList.html
index c000dff6f8c..8c367976da2 100644
--- a/src/main/java/org/olat/course/nodes/edubase/_content/bookSectionList.html
+++ b/src/main/java/org/olat/course/nodes/edubase/_content/bookSectionList.html
@@ -20,10 +20,8 @@
 						<div>$r.render(${bookSection.getDownLinkEl().getComponent().getComponentName()})</div>
 					#end
 				</td>
-				<td class="o_edubase_bs_cover" calss="cover${bookSection.getBookIdEl().getValue()}">
-					<object data="$!{bookSection.getCoverUrl()}">
-  						<img src="$r.staticLink('images/edubase/empty_cover.png')" alt="COVER">
-					</object>
+				<td class="o_edubase_bs_cover">
+					<img src="$!{bookSection.getBookSection().getCoverUrl()}" onerror="if (this.src != '$r.staticLink('images/edubase/empty_cover.png')') this.src = '$r.staticLink('images/edubase/empty_cover.png')';" />
 				</td>
 			</tbody>
 		</table>
diff --git a/src/main/java/org/olat/course/nodes/edubase/_content/overview_description_disabled.html b/src/main/java/org/olat/course/nodes/edubase/_content/overview_description_disabled.html
index ed0d200245b..55eb23de7bb 100644
--- a/src/main/java/org/olat/course/nodes/edubase/_content/overview_description_disabled.html
+++ b/src/main/java/org/olat/course/nodes/edubase/_content/overview_description_disabled.html
@@ -2,9 +2,7 @@
 #foreach($bookSection in $bookSections)
 	<div class="o_edubase_booksection">
 		#set( $runindex = $run + $foreach.index)
-		<object data="$!{bookSection.getCoverUrl()}">
-  			<img src="$r.staticLink('images/edubase/empty_cover.png')" alt="COVER" onClick="$r.javaScriptCommand($runindex);">
-		</object>
+		<img src="$!{bookSection.getCoverUrl()}" onerror="if (this.src != '$r.staticLink('images/edubase/empty_cover.png')') this.src = '$r.staticLink('images/edubase/empty_cover.png')';">
 		<div>
 			$r.escapeHtml($bookSection.getTitle())
 		</div>
diff --git a/src/main/java/org/olat/course/nodes/edubase/_content/overview_description_enabled.html b/src/main/java/org/olat/course/nodes/edubase/_content/overview_description_enabled.html
index 518d383b4e6..eb3740b9f42 100644
--- a/src/main/java/org/olat/course/nodes/edubase/_content/overview_description_enabled.html
+++ b/src/main/java/org/olat/course/nodes/edubase/_content/overview_description_enabled.html
@@ -2,9 +2,7 @@
 #foreach($bookSection in $bookSections)
 <div class="row">
 	<div class="col-sm-2 o_edubase_cover o_block_top">
-		<object data="$!{bookSection.getCoverUrl()}">
-  			<img src="$r.staticLink('images/edubase/empty_cover.png')" alt="COVER">
-		</object>
+		<img src="$!{bookSection.getCoverUrl()}" onerror="if (this.src != '$r.staticLink('images/edubase/empty_cover.png')') this.src = '$r.staticLink('images/edubase/empty_cover.png')';"/>
 	</div>
 	<div class="col-sm-10 o_edubase_booksection">
 		<div class="row">
diff --git a/src/main/java/org/olat/course/nodes/edubase/_content/peekview.html b/src/main/java/org/olat/course/nodes/edubase/_content/peekview.html
index 56991201e88..bcc4afbada7 100644
--- a/src/main/java/org/olat/course/nodes/edubase/_content/peekview.html
+++ b/src/main/java/org/olat/course/nodes/edubase/_content/peekview.html
@@ -2,9 +2,7 @@
 	#foreach($bookSection in $bookSections)
 		<div class="o_edubase_pv_fig">
 			<figure>
-				<object data="$!{bookSection.getCoverUrl()}">
-  					<img src="$r.staticLink('images/edubase/empty_cover.png')" alt="COVER">
-				</object>
+				<img src="$!{bookSection.getCoverUrl()}" onerror="if (this.src != '$r.staticLink('images/edubase/empty_cover.png')') this.src = '$r.staticLink('images/edubase/empty_cover.png')';" />
 				<figcaption>$r.escapeHtml($bookSection.getTitle())</figcaption>
 			</figure>
 		</div>
diff --git a/src/main/java/org/olat/course/nodes/edubase/_i18n/LocalStrings_de.properties b/src/main/java/org/olat/course/nodes/edubase/_i18n/LocalStrings_de.properties
index 076b49787f9..e69a2661215 100644
--- a/src/main/java/org/olat/course/nodes/edubase/_i18n/LocalStrings_de.properties
+++ b/src/main/java/org/olat/course/nodes/edubase/_i18n/LocalStrings_de.properties
@@ -18,6 +18,7 @@ edubase.book.section.page.from.to=Seite {0} bis {1}
 edubase.book.section.page.to=Seite bis
 edubase.book.section.title=Buchtitel
 edubase.with.description.enabled=Leseauftrag
+form.error.page.to.higher.from=Seite bis muss gr\u00F6sser sein als Seite von
 form.error.wrong.int=Falsches Zahlenformat. Beispiele\: 2, 10, 144
 form.error.wrong.section.id=Ung\u00FCltiger Wert
 link.text=Edubase
diff --git a/src/main/java/org/olat/course/nodes/edubase/_i18n/LocalStrings_en.properties b/src/main/java/org/olat/course/nodes/edubase/_i18n/LocalStrings_en.properties
index 69825430fa4..7c4176cb63e 100644
--- a/src/main/java/org/olat/course/nodes/edubase/_i18n/LocalStrings_en.properties
+++ b/src/main/java/org/olat/course/nodes/edubase/_i18n/LocalStrings_en.properties
@@ -18,6 +18,7 @@ edubase.book.section.page.from.to=Page {0} to {1}
 edubase.book.section.page.to=Page to
 edubase.book.section.title=Book title
 edubase.with.description.enabled=Reading order
+form.error.page.to.higher.from=Page to has to be higher than page from
 form.error.wrong.int=Wrong numeral format. Examples\: 2, 10, 144
 form.error.wrong.section.id=Invalid value
 link.text=Edubase
diff --git a/src/main/java/org/olat/modules/edubase/manager/EdubaseManagerImpl.java b/src/main/java/org/olat/modules/edubase/manager/EdubaseManagerImpl.java
index 77230e02a43..11463e6d503 100644
--- a/src/main/java/org/olat/modules/edubase/manager/EdubaseManagerImpl.java
+++ b/src/main/java/org/olat/modules/edubase/manager/EdubaseManagerImpl.java
@@ -22,25 +22,17 @@ package org.olat.modules.edubase.manager;
 
 import java.io.EOFException;
 import java.net.SocketTimeoutException;
-import java.security.KeyManagementException;
-import java.security.KeyStoreException;
-import java.security.NoSuchAlgorithmException;
-import java.security.cert.CertificateException;
 import java.util.Optional;
 
 import org.apache.http.client.config.RequestConfig;
 import org.apache.http.client.methods.CloseableHttpResponse;
 import org.apache.http.client.methods.HttpGet;
-import org.apache.http.conn.ssl.AllowAllHostnameVerifier;
-import org.apache.http.conn.ssl.SSLContextBuilder;
-import org.apache.http.conn.ssl.TrustStrategy;
 import org.apache.http.impl.client.CloseableHttpClient;
 import org.apache.http.impl.client.HttpClients;
 import org.apache.http.util.EntityUtils;
 import org.codehaus.jackson.JsonParseException;
 import org.codehaus.jackson.map.ObjectMapper;
 import org.olat.basesecurity.AuthHelper;
-import org.olat.core.helpers.Settings;
 import org.olat.core.id.IdentityEnvironment;
 import org.olat.core.logging.OLog;
 import org.olat.core.logging.Tracing;
@@ -158,9 +150,9 @@ public class EdubaseManagerImpl implements EdubaseManager {
 		String url = String.format(edubaseModule.getInfoverUrl(), bookId);
 		HttpGet request = new HttpGet(url);
 		request.setConfig(requestConfig);
-		try (CloseableHttpClient httpClient = createAllwaysTrustingHttpClient();
+		try (CloseableHttpClient httpClient = HttpClients.createDefault();
 				CloseableHttpResponse httpResponse = httpClient.execute(request);) {
-			String json = EntityUtils.toString(httpResponse.getEntity());
+			String json = EntityUtils.toString(httpResponse.getEntity(), "UTF-8");
 			ObjectMapper objectMapper = new ObjectMapper();
 			infoReponse = objectMapper.readValue(json, BookDetailsImpl.class);
 		} catch (SocketTimeoutException socketTimeoutException) {
@@ -171,27 +163,8 @@ public class EdubaseManagerImpl implements EdubaseManager {
 		} catch(Exception e) {
 			log.error("", e);
 		}
-
+		
 		return infoReponse;
 	}
 
-	private CloseableHttpClient createAllwaysTrustingHttpClient()
-			throws NoSuchAlgorithmException, KeyManagementException, KeyStoreException {
-		CloseableHttpClient httpClient;
-		if (Settings.isDebuging()) {
-			httpClient = HttpClients.custom()
-					.setHostnameVerifier(new AllowAllHostnameVerifier())
-					.setSslcontext(new SSLContextBuilder().loadTrustMaterial(null, new TrustStrategy() {
-						@Override
-						public boolean isTrusted(java.security.cert.X509Certificate[] chain, String authType)
-								throws CertificateException {
-							return true;
-						}
-					}).build()).build();
-		} else {
-			httpClient = HttpClients.createDefault();
-		}
-		return httpClient;
-	}
-
 }
-- 
GitLab