Skip to content

Commit fa25e53

Browse files
fabrizzio-dotCMSfabrizzio-dotCMS
authored
fix(Page) Working/Live Images loading inconsistenly Refs: 31362 (dotCMS#31408)
### Proposed Changes * Fix setting incorrect PageMode on VelocityServlet. * When switching to LIVE an incoming request to the Velocity servlet was changing the PageMode at the session level to PREVIEW_MODE, causing a weird behavior when switching images * BinaryExportServlet isn't supposed to cache images for the admin, however, it was doing it, because of a condition like. if(!isAdmin) {serve the image with cache} else { do not use cache to serve the image} The general rule is if we're in admin mode we do not serve images using cache. The bug was that when serving content in LIVE mode from within the UVE it would assume we were not on admin therefore the images would stay in cache
1 parent a1b50b1 commit fa25e53

File tree

8 files changed

+225
-62
lines changed

8 files changed

+225
-62
lines changed

‎dotCMS/src/main/java/com/dotcms/rendering/velocity/servlet/VelocityServlet.java‎

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -26,15 +26,14 @@
2626
import com.dotmarketing.util.WebKeys;
2727
import com.google.common.annotations.VisibleForTesting;
2828
import com.liferay.portal.model.User;
29-
import javax.servlet.http.HttpSession;
30-
import org.apache.velocity.exception.ResourceNotFoundException;
31-
29+
import java.io.IOException;
3230
import javax.servlet.ServletConfig;
3331
import javax.servlet.ServletException;
3432
import javax.servlet.http.HttpServlet;
3533
import javax.servlet.http.HttpServletRequest;
3634
import javax.servlet.http.HttpServletResponse;
37-
import java.io.IOException;
35+
import javax.servlet.http.HttpSession;
36+
import org.apache.velocity.exception.ResourceNotFoundException;
3837

3938
public class VelocityServlet extends HttpServlet {
4039

@@ -70,24 +69,37 @@ public static PageMode processPageMode (final User user, final HttpServletReques
7069
}
7170

7271
if (LoginMode.UNKNOWN == loginMode) {
73-
74-
return user.isFrontendUser()
75-
? PageMode.setPageMode(request, PageMode.LIVE)
76-
: useNavigateMode(request, loginMode)
77-
? PageMode.setPageMode(request, PageMode.NAVIGATE_EDIT_MODE)
78-
: PageMode.setPageMode(request, PageMode.LIVE);
72+
return determinePageMode(request, user, LoginMode.UNKNOWN);
7973
}
8074

8175
if ( LoginMode.FE == loginMode) {
82-
return PageMode.setPageMode(request, PageMode.LIVE);
76+
return PageMode.setPageMode(request, PageMode.LIVE, false);
8377
}
8478

8579
return useNavigateMode(request, loginMode) ?
86-
PageMode.setPageMode(request, PageMode.NAVIGATE_EDIT_MODE) : PageMode.setPageMode(request, PageMode.PREVIEW_MODE);
80+
PageMode.setPageMode(request, PageMode.NAVIGATE_EDIT_MODE, false) : PageMode.setPageMode(request, PageMode.PREVIEW_MODE, false);
8781
}
8882

89-
private static boolean useNavigateMode(final HttpServletRequest request, LoginMode loginMode) {
83+
/**
84+
* This method will determine the page mode based on the user and the login mode
85+
* @param request HttpServletRequest
86+
* @param user User
87+
* @param loginMode LoginMode
88+
* @return PageMode
89+
*/
90+
private static PageMode determinePageMode(HttpServletRequest request, User user, LoginMode loginMode) {
91+
if (user.isFrontendUser()) {
92+
return PageMode.setPageMode(request, PageMode.LIVE, false);
93+
}
9094

95+
if (useNavigateMode(request, loginMode)) {
96+
return PageMode.setPageMode(request, PageMode.NAVIGATE_EDIT_MODE,false);
97+
}
98+
99+
return PageMode.setPageMode(request, PageMode.LIVE, false);
100+
}
101+
102+
private static boolean useNavigateMode(final HttpServletRequest request, LoginMode loginMode) {
91103

92104
if (LoginMode.FE == loginMode) {
93105
return false;
@@ -161,7 +173,9 @@ protected final void service(HttpServletRequest req, HttpServletResponse respons
161173
request,
162174
response
163175
);
176+
164177
Logger.debug(this, "VelocityServlet_service pageHtml: " + pageHtml);
178+
165179
response.getOutputStream().write(pageHtml.getBytes());
166180
} catch (ResourceNotFoundException rnfe) {
167181
Logger.warnAndDebug(this.getClass(), "ResourceNotFoundException" + rnfe.toString(), rnfe);

‎dotCMS/src/main/java/com/dotmarketing/filters/TimeMachineFilter.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
8080
if(req.getSession().getAttribute("tm_date")!=null && urlUtil.amISomething(uri,(Host)req.getSession().getAttribute("tm_host")
8181
,Long.parseLong((String)req.getSession().getAttribute("tm_lang")))) {
8282
com.liferay.portal.model.User user = null;
83-
PageMode.setPageMode(req, PageMode.PREVIEW_MODE);
83+
PageMode.setPageMode(req, PageMode.PREVIEW_MODE,false);
8484
try {
8585
user = com.liferay.portal.util.PortalUtil.getUser((HttpServletRequest) request);
8686
if(!APILocator.getLayoutAPI().doesUserHaveAccessToPortlet("time-machine", user)){

‎dotCMS/src/main/java/com/dotmarketing/portlets/htmlpageasset/business/render/HTMLPageAssetRenderedAPIImpl.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ public PageView getPageRendered(
195195

196196
final PageMode mode = context.getPageMode();
197197

198-
PageMode.setPageMode(request, mode);
198+
PageMode.setPageMode(request, mode, false);
199199

200200
final Host host = this.hostWebAPI.getCurrentHost(request, context.getUser());
201201
final HTMLPageUrl htmlPageUrl = context.getPage() != null

‎dotCMS/src/main/java/com/dotmarketing/servlets/BinaryExporterServlet.java‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -466,10 +466,9 @@ public void doGet(HttpServletRequest req, HttpServletResponse resp) throws Servl
466466

467467
} else {
468468

469-
470-
471-
// Set the expiration time
472-
if (!mode.isAdmin) {
469+
// Set the expiration time
470+
// Sometimes when The PageMode flag is LIVE, and we are serving a live view of the page from within the admin tool we might end up caching images that we shouldn't
471+
if (!isDotAdminRequest(req) && !mode.isAdmin) {
473472

474473
int _daysCache = 365;
475474
GregorianCalendar expiration = new GregorianCalendar();
@@ -685,6 +684,16 @@ public void doGet(HttpServletRequest req, HttpServletResponse resp) throws Servl
685684

686685
}
687686

687+
/**
688+
* Test the request to see if it is a dotAdmin request
689+
* @param request the request to test
690+
* @return true if the request is a dotAdmin request
691+
*/
692+
public static boolean isDotAdminRequest(HttpServletRequest request) {
693+
final String referer = request.getHeader("referer");
694+
return referer != null && referer.contains("/dotAdmin");
695+
}
696+
688697
private Contentlet getContentletByIdentifier(final PageMode pageMode,
689698
final String identifier, final long languageId, final User user)
690699
throws DotDataException, DotSecurityException {

‎dotCMS/src/main/java/com/dotmarketing/util/PageMode.java‎

Lines changed: 17 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -119,51 +119,36 @@ public static PageMode setPageMode(final HttpServletRequest request, boolean con
119119
* @param mode
120120
* @return
121121
*/
122-
public static PageMode setPageMode(final HttpServletRequest request, final PageMode mode) {
122+
public static PageMode setPageMode(final HttpServletRequest request, final PageMode mode){
123+
return setPageMode(request, mode, true);
124+
}
123125

126+
/**
127+
* Page mode can only be set for back end users, not for front end users (even logged in Front end users)
128+
* We should avoid setting the mode in the session.
129+
* Session should be used to keep immutable data. like the user of company. But not state data that might dictate the behavior of the UI.
130+
* I'm introducing this method to avoid setting the mode in the session.
131+
* Many times passing stuff down the request is enough.
132+
* @param request HttpServletRequest
133+
* @param mode PageMode to set
134+
* @param setSession if true, the mode will be set in the session
135+
* @return
136+
*/
137+
public static PageMode setPageMode(final HttpServletRequest request, final PageMode mode, final boolean setSession) {
124138
if (DEFAULT_PAGE_MODE != mode) {
125139
final User user = PortalUtil.getUser(request);
126140
if (user == null || !user.isBackendUser()) {
127141
return DEFAULT_PAGE_MODE;
128142
}
129143
}
130-
131-
if(request.getSession(false)!=null) {
144+
//here we're saying that... we want to set page mode in the request and not override the session actual value
145+
if( setSession && null != request.getSession(false)) {
132146
request.getSession().setAttribute(WebKeys.PAGE_MODE_SESSION, mode);
133147
}
134148
request.setAttribute(WebKeys.PAGE_MODE_PARAMETER, mode);
135149
return mode;
136150
}
137151

138-
private static boolean isPageModeSet(final HttpSession ses) {
139-
return (ses != null && ses.getAttribute(com.dotmarketing.util.WebKeys.PAGE_MODE_SESSION) != null);
140-
}
141-
142-
private static PageMode getCurrentPageMode(final HttpSession ses) {
143-
PageMode sessionPageMode = ses==null ? DEFAULT_PAGE_MODE : (PageMode) ses.getAttribute(WebKeys.PAGE_MODE_SESSION);
144-
145-
if (isNavigateEditMode(ses)) {
146-
return PageMode.NAVIGATE_EDIT_MODE;
147-
} else {
148-
return sessionPageMode;
149-
}
150-
}
151-
152-
private static boolean isNavigateEditMode(final HttpSession ses) {
153-
PageMode sessionPageMode = ses==null ? DEFAULT_PAGE_MODE : (PageMode) ses.getAttribute(WebKeys.PAGE_MODE_SESSION);
154-
HttpServletRequest request = HttpServletRequestThreadLocal.INSTANCE.getRequest();
155-
156-
final User user = PortalUtil.getUser(request);
157-
if (user == null || !user.isBackendUser()) {
158-
return false;
159-
}
160-
161-
162-
return sessionPageMode != PageMode.LIVE &&
163-
request != null &&
164-
request.getAttribute(WebKeys.PAGE_MODE_PARAMETER) == null ;
165-
}
166-
167152
/**
168153
* Checks if the current Page Mode belongs to Edit Mode.
169154
*

‎dotcms-integration/src/test/java/com/dotcms/rendering/velocity/servlet/VelocityServletIntegrationTest.java‎

Lines changed: 95 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,8 @@
6565
import static com.dotcms.datagen.TestDataUtils.getNewsLikeContentType;
6666
import static com.dotmarketing.util.WebKeys.LOGIN_MODE_PARAMETER;
6767
import static org.junit.Assert.assertEquals;
68+
import static org.junit.Assert.assertNotNull;
69+
import static org.junit.Assert.assertNull;
6870
import static org.mockito.ArgumentMatchers.any;
6971
import static org.mockito.ArgumentMatchers.eq;
7072
import static org.mockito.ArgumentMatchers.anyInt;
@@ -592,6 +594,49 @@ private void testServerPageFor(final User user, final LoginMode mode)
592594
verifyPageServed(pageContent, outputStream);
593595
}
594596

597+
/**
598+
* Method to test: {@link VelocityServlet#service(HttpServletRequest, HttpServletResponse)}
599+
* Given scenario: A backend user is logged in and request a page in preview mode
600+
* Expected result: The page should be served in preview mode and the attribute should be set at request level never at session level
601+
* @throws Exception
602+
*/
603+
@Test
604+
public void Test_Page_Mode_Scope() throws Exception {
605+
606+
final User loginUser = mock(User.class);
607+
when(loginUser.hasConsoleAccess()).thenReturn(true);
608+
when(loginUser.isAnonymousUser()).thenReturn(false);
609+
when(loginUser.isBackendUser()).thenReturn(true);
610+
when(loginUser.isFrontendUser()).thenReturn(false);
611+
when(loginUser.isActive()).thenReturn(true);
612+
613+
final HttpServletRequest mockRequest = createMockRequest("/dotAdmin/blog/index",
614+
loginUser, LoginMode.BE, true);
615+
616+
final ServletOutputStream outputStream = mock(ServletOutputStream.class);
617+
when(response.getOutputStream()).thenReturn(outputStream);
618+
619+
final HTMLPageAssetRenderedAPI pageAssetRenderedAPI = createHtmlPageAssetRenderedAPIMock(
620+
loginUser, "<html>lol</html>", PageMode.PREVIEW_MODE);
621+
622+
final VelocityServlet velocityServlet = new VelocityServlet(WebAPILocator.getUserWebAPI(),
623+
pageAssetRenderedAPI);
624+
velocityServlet.service(mockRequest, response);
625+
626+
verifyPageServed("<html>lol</html>", outputStream);
627+
628+
//Page Mode can be recovered from the PageMode utility class
629+
assertEquals("Page Mode should be returned accurately by PageMode.get", PageMode.PREVIEW_MODE, PageMode.get(mockRequest));
630+
//The attribute can be recovered at request level consistently
631+
assertEquals("Test the attribute only exists at request level", PageMode.PREVIEW_MODE, mockRequest.getAttribute(WebKeys.PAGE_MODE_PARAMETER));
632+
633+
//The attribute should not be set at session level
634+
final HttpSession session = mockRequest.getSession(false);
635+
assertNotNull(session);
636+
assertNull(session.getAttribute(WebKeys.PAGE_MODE_SESSION));
637+
638+
}
639+
595640
private static void verifyPageServed(final String pageContent, final ServletOutputStream outputStream)
596641
throws IOException {
597642
verify(outputStream, times(1)).write(pageContent.getBytes());
@@ -600,19 +645,66 @@ private static void verifyPageServed(final String pageContent, final ServletOutp
600645
public HttpServletRequest createMockRequest(final String referer, final User user, final LoginMode loginMode) {
601646
return createMockRequest(referer, user, loginMode, false);
602647
}
648+
603649
public HttpServletRequest createMockRequest(final String referer, final User user,
604650
final LoginMode loginMode, final boolean disabledNavigateMode) {
605651

652+
// Create the mock of HttpServletRequest (VelocityRequestWrapper simulated)
606653
VelocityRequestWrapper velocityRequest = mock(VelocityRequestWrapper.class);
654+
655+
// Map to store request attributes
656+
final Map<String, Object> requestAttributes = new HashMap<>();
657+
658+
// Preload basic request attributes
659+
requestAttributes.put("requestURI", "/lol");
660+
requestAttributes.put(com.liferay.portal.util.WebKeys.USER, user);
661+
requestAttributes.put("disabledNavigateMode", String.valueOf(disabledNavigateMode));
662+
663+
// Configure dynamic behavior of setAttribute and getAttribute for the request
664+
doAnswer(invocation -> {
665+
String key = invocation.getArgument(0);
666+
Object value = invocation.getArgument(1);
667+
requestAttributes.put(key, value);
668+
return null;
669+
}).when(velocityRequest).setAttribute(anyString(), any());
670+
671+
when(velocityRequest.getAttribute(anyString())).thenAnswer(invocation -> {
672+
String key = invocation.getArgument(0);
673+
return requestAttributes.get(key);
674+
});
675+
676+
// Simulate getRequestURI with a direct response
607677
when(velocityRequest.getRequestURI()).thenReturn("/lol");
608-
when(velocityRequest.getAttribute(com.liferay.portal.util.WebKeys.USER)).thenReturn(user);
678+
679+
// Simulate getParameter directly
680+
when(velocityRequest.getParameter("disabledNavigateMode"))
681+
.thenReturn(String.valueOf(disabledNavigateMode));
682+
683+
// Map to store session attributes
684+
final Map<String, Object> sessionAttributes = new HashMap<>();
685+
686+
// Create the mock for HttpSession
609687
final HttpSession session = mock(HttpSession.class);
610-
when(session.getAttribute(LOGIN_MODE_PARAMETER)).thenReturn(loginMode);
611688
when(velocityRequest.getSession(Mockito.anyBoolean())).thenReturn(session);
612689
when(velocityRequest.getSession()).thenReturn(session);
613690

614-
when(velocityRequest.getParameter("disabledNavigateMode")).thenReturn(String.valueOf(disabledNavigateMode));
691+
// Preload the loginMode attribute in the session
692+
sessionAttributes.put(LOGIN_MODE_PARAMETER, loginMode);
693+
694+
// Configure dynamic behavior of setAttribute and getAttribute for the session
695+
doAnswer(invocation -> {
696+
String key = invocation.getArgument(0);
697+
Object value = invocation.getArgument(1);
698+
sessionAttributes.put(key, value);
699+
return null;
700+
}).when(session).setAttribute(anyString(), any());
701+
702+
when(session.getAttribute(anyString())).thenAnswer(invocation -> {
703+
String key = invocation.getArgument(0);
704+
return sessionAttributes.get(key);
705+
});
615706

707+
// Set the referer header if provided
616708
if (referer != null) {
617709
when(velocityRequest.getHeader("referer")).thenReturn(referer);
618710
}

0 commit comments

Comments
 (0)