Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Here is another idea for taming robot indexers #4016

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions api/src/org/labkey/api/action/PermissionCheckableAction.java
Original file line number Diff line number Diff line change
Expand Up @@ -174,9 +174,12 @@ private void _checkActionPermissions(Set<Role> contextualRoles) throws Unauthori
}

boolean requiresLogin = actionClass.isAnnotationPresent(RequiresLogin.class);
if (requiresLogin && user.isGuest())
if (requiresLogin)
{
throw new UnauthorizedException();
if (user.isGuest())
throw new UnauthorizedException();
if (this instanceof BaseViewAction<?> viewaction)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we expect this to be common? Won't crawlers always be guests?

Note there are a number of unit test failures when there's not a full HTTP request in progress: https://teamcity.labkey.org/buildConfiguration/LabkeyTrunk_BvtBPostgres/2225932?hideProblemsFromDependencies=false&hideTestsFromDependencies=false&expandBuildChangesSection=true&expandBuildTestsSection=true

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking about how we might use annotations as hints (or explicitly). I agree this is not functional...

viewaction.getPageConfig().setNoIndex();
}

// User must have ALL permissions in this set
Expand Down
29 changes: 21 additions & 8 deletions api/src/org/labkey/api/view/template/PageConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
import static java.util.Objects.requireNonNullElse;
import static org.apache.commons.lang3.StringUtils.isNotBlank;
import static org.apache.commons.lang3.StringUtils.isNotEmpty;
import static org.labkey.api.data.DataRegion.CONTAINER_FILTER_NAME;
import static org.labkey.api.util.PageFlowUtil.jsString;
import static org.labkey.api.view.template.WarningService.SESSION_WARNINGS_BANNER_KEY;

Expand Down Expand Up @@ -401,7 +402,7 @@ public void setCanonicalLink(String link)
}


String[] ignoreParameters = new String[] {"_dc", "_template", "_print", "_debug", "_docid", DataRegion.LAST_FILTER_PARAM};
static final Set<String> ignoreParameters = Set.of("_dc", "_template", "_print", "_debug", "_docid", DataRegion.LAST_FILTER_PARAM);

@Nullable
private String getCanonicalLink(URLHelper current)
Expand All @@ -410,17 +411,29 @@ private String getCanonicalLink(URLHelper current)
return _canonicalLink;
if (null == current)
return null;
URLHelper u = null;
if (current instanceof ActionURL && !((ActionURL)current).isCanonical())
u = current.clone();
for (String p : ignoreParameters)
return makeCanonicalLink(current);
}


private static String makeCanonicalLink(URLHelper current)
{
var params = current.getParameters();
URLHelper u = current.clone().deleteParameters();

for (var pair : params)
{
if (null != current.getParameter(p))
u = (null==u ? current.clone() : u).deleteParameter(p);
if (ignoreParameters.contains(pair.getKey()))
continue;
// Strip container filters from the URL to prevent crawlers from over-indexing a URL with different parameter values
if (pair.getKey().endsWith(CONTAINER_FILTER_NAME))
labkey-matthewb marked this conversation as resolved.
Show resolved Hide resolved

continue;
u.addParameter(pair.getKey(), pair.getValue());
}
return null == u ? null : u.getURIString();
return u.getURIString();
}


public HtmlString getPreloadTags()
{
final List<String> fonts = List.of(
Expand Down
11 changes: 11 additions & 0 deletions core/src/org/labkey/core/admin/AdminController.java
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@
import org.springframework.web.servlet.mvc.Controller;

import javax.mail.MessagingException;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import java.awt.*;
Expand Down Expand Up @@ -290,6 +291,16 @@ public class AdminController extends SpringActionController
private static long _errorMark = 0;
private static long _primaryLogMark = 0;


@Override
protected void beforeAction(Controller action) throws ServletException
{
super.beforeAction(action);
if (action instanceof BaseViewAction<?> viewaction)
viewaction.getPageConfig().setNoIndex();
}


public static void registerAdminConsoleLinks()
{
Container root = ContainerManager.getRoot();
Expand Down