From 774776d054b0a4f05511f1625bd011b404a1e3fb Mon Sep 17 00:00:00 2001 From: Luke Sandberg Date: Fri, 20 Dec 2024 16:18:46 -0800 Subject: [PATCH] Add a boolean parameter to `flushPendingLoggingAttributes` to track whether we are writing to an `anchor` tag or not. Add support for `flushPendingLoggingParameters` in all our appendable implementations PiperOrigin-RevId: 708454348 --- .../BasicDirectivesRuntime.java | 5 ++++ .../coredirectives/CoreDirectivesRuntime.java | 11 +++++++ .../ForwardingLoggingAdvisingAppendable.java | 7 +++++ .../soy/data/LoggingAdvisingAppendable.java | 25 +++++++++++++--- .../soy/jbcsrc/AppendableExpression.java | 7 +++-- .../template/soy/jbcsrc/SoyNodeCompiler.java | 8 +++-- .../soy/jbcsrc/api/OutputAppendable.java | 6 ++-- .../soy/jbcsrc/runtime/JbcSrcRuntime.java | 5 ++++ .../AddFlushPendingLoggingAttributesPass.java | 11 +++++++ .../soy/shared/internal/BuiltinFunction.java | 5 +++- .../soy/shared/internal/Sanitizers.java | 30 +++++++++++++++++++ .../internal/StreamingAttributeEscaper.java | 11 +++++++ .../soy/shared/internal/StreamingEscaper.java | 5 ++++ .../jbcsrc/StreamingPrintDirectivesTest.java | 8 ++++- 14 files changed, 130 insertions(+), 14 deletions(-) diff --git a/java/src/com/google/template/soy/basicdirectives/BasicDirectivesRuntime.java b/java/src/com/google/template/soy/basicdirectives/BasicDirectivesRuntime.java index 86703af2cf..70bc0a6d7c 100644 --- a/java/src/com/google/template/soy/basicdirectives/BasicDirectivesRuntime.java +++ b/java/src/com/google/template/soy/basicdirectives/BasicDirectivesRuntime.java @@ -147,6 +147,11 @@ public LoggingAdvisingAppendable appendLoggingFunctionInvocation( return this; } + @Override + public LoggingAdvisingAppendable flushPendingLoggingAttributes(boolean isAnchorTag) { + return this; + } + @Override public boolean softLimitReached() { return false; diff --git a/java/src/com/google/template/soy/coredirectives/CoreDirectivesRuntime.java b/java/src/com/google/template/soy/coredirectives/CoreDirectivesRuntime.java index 2b216565f0..dedbbbaf42 100644 --- a/java/src/com/google/template/soy/coredirectives/CoreDirectivesRuntime.java +++ b/java/src/com/google/template/soy/coredirectives/CoreDirectivesRuntime.java @@ -26,6 +26,7 @@ import com.google.template.soy.data.restricted.NullData; import com.google.template.soy.shared.internal.AbstractStreamingHtmlEscaper; import com.google.template.soy.shared.internal.EscapingConventions; +import java.io.IOException; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -101,6 +102,16 @@ public LoggingAdvisingAppendable exitLoggableElement() { } return this; } + + @CanIgnoreReturnValue + @Override + public LoggingAdvisingAppendable flushPendingLoggingAttributes(boolean isAnchorTag) + throws IOException { + if (isInHtml()) { + delegate.flushPendingLoggingAttributes(isAnchorTag); + } + return this; + } } private CoreDirectivesRuntime() {} diff --git a/java/src/com/google/template/soy/data/ForwardingLoggingAdvisingAppendable.java b/java/src/com/google/template/soy/data/ForwardingLoggingAdvisingAppendable.java index eaebb9d2d6..e9ed7eb812 100644 --- a/java/src/com/google/template/soy/data/ForwardingLoggingAdvisingAppendable.java +++ b/java/src/com/google/template/soy/data/ForwardingLoggingAdvisingAppendable.java @@ -86,6 +86,13 @@ public LoggingAdvisingAppendable appendLoggingFunctionInvocation( return this; } + @Override + public LoggingAdvisingAppendable flushPendingLoggingAttributes(boolean isAnchorTag) + throws IOException { + delegate.flushPendingLoggingAttributes(isAnchorTag); + return this; + } + @Override public void flushBuffers(int depth) throws IOException { if (depth > 0) { diff --git a/java/src/com/google/template/soy/data/LoggingAdvisingAppendable.java b/java/src/com/google/template/soy/data/LoggingAdvisingAppendable.java index 252f7fe428..0aef63c0ed 100644 --- a/java/src/com/google/template/soy/data/LoggingAdvisingAppendable.java +++ b/java/src/com/google/template/soy/data/LoggingAdvisingAppendable.java @@ -97,10 +97,10 @@ public abstract LoggingAdvisingAppendable append(CharSequence csq, int start, in public abstract LoggingAdvisingAppendable exitLoggableElement(); /** Flushes all pending logging attributes. */ - public final LoggingAdvisingAppendable flushPendingLoggingAttributes() { - // TODO(b/383661457): Implement this. - return this; - } + @CanIgnoreReturnValue + @Nonnull + public abstract LoggingAdvisingAppendable flushPendingLoggingAttributes(boolean isAnchorTag) + throws IOException; /** * Flushes all intermediate buffers stored within the appendable. @@ -245,11 +245,15 @@ public String toString() { public static class BufferingAppendable extends LoggingAdvisingAppendable { private static final Object EXIT_LOG_STATEMENT_MARKER = new Object(); + private static final Object FLUSH_PENDING_LOGGING_ATTRBUTES_TRUE_MARKER = new Object(); + private static final Object FLUSH_PENDING_LOGGING_ATTRBUTES_FALSE_MARKER = new Object(); // lazily allocated list that contains one of 7 types of objects, each which corresponds to one // of the callback methods. // - String literal string content -> corresponds to a contiguous sequence of append calls // - LogStatement -> corresponds to enterLoggableElement // - EXIT_LOG_STATEMENT_MARKER -> corresponds to exitLoggableElement + // - FLUSH_PENDING_LOGGING_(TRUE|FALSE)_ATTRBUTES_MARKER -> corresponds to + // flushPendingLoggingAttributes(true|false) // - LoggingFunctionInvocation -> corresponds to appendLoggingFunctionInvocation private List commands; private final StringBuilder builder = new StringBuilder(); @@ -329,6 +333,16 @@ public LoggingAdvisingAppendable exitLoggableElement() { return this; } + @Override + public LoggingAdvisingAppendable flushPendingLoggingAttributes(boolean isAnchor) { + getCommandsAndAddPendingStringData() + .add( + isAnchor + ? FLUSH_PENDING_LOGGING_ATTRBUTES_TRUE_MARKER + : FLUSH_PENDING_LOGGING_ATTRBUTES_FALSE_MARKER); + return this; + } + @CanIgnoreReturnValue @Override public LoggingAdvisingAppendable appendLoggingFunctionInvocation( @@ -369,6 +383,9 @@ private static void replayCommandOn(Object o, LoggingAdvisingAppendable appendab ((LoggingFunctionCommand) o).replayOn(appendable); } else if (o == EXIT_LOG_STATEMENT_MARKER) { appendable.exitLoggableElement(); + } else if (o == FLUSH_PENDING_LOGGING_ATTRBUTES_TRUE_MARKER + || o == FLUSH_PENDING_LOGGING_ATTRBUTES_FALSE_MARKER) { + appendable.flushPendingLoggingAttributes(o == FLUSH_PENDING_LOGGING_ATTRBUTES_TRUE_MARKER); } else if (o instanceof LogStatement) { appendable.enterLoggableElement((LogStatement) o); } else { diff --git a/java/src/com/google/template/soy/jbcsrc/AppendableExpression.java b/java/src/com/google/template/soy/jbcsrc/AppendableExpression.java index c0684ff72c..466ea761c9 100644 --- a/java/src/com/google/template/soy/jbcsrc/AppendableExpression.java +++ b/java/src/com/google/template/soy/jbcsrc/AppendableExpression.java @@ -164,7 +164,8 @@ static Statement concat(List statements) { MethodRef.createNonPure(LoggingAdvisingAppendable.class, "flushBuffers", int.class); private static final MethodRef FLUSH_PENDING_LOGGING_ATTRBIUTES = - MethodRef.createNonPure(LoggingAdvisingAppendable.class, "flushPendingLoggingAttributes"); + MethodRef.createNonPure( + LoggingAdvisingAppendable.class, "flushPendingLoggingAttributes", boolean.class); static AppendableExpression forExpression(Expression delegate) { return new AppendableExpression(delegate, e -> e, /* supportsSoftLimiting= */ true); @@ -271,8 +272,8 @@ AppendableExpression setSanitizedContentKindAndDirectionality(SanitizedContentKi BytecodeUtils.constantSanitizedContentKindAsContentKind(kind))); } - AppendableExpression flushPendingLoggingAttributes() { - return withNewDelegate(e -> e.invoke(FLUSH_PENDING_LOGGING_ATTRBIUTES)); + AppendableExpression flushPendingLoggingAttributes(boolean isAnchorTag) { + return withNewDelegate(e -> e.invoke(FLUSH_PENDING_LOGGING_ATTRBIUTES, constant(isAnchorTag))); } Statement flushBuffers(int depth) { diff --git a/java/src/com/google/template/soy/jbcsrc/SoyNodeCompiler.java b/java/src/com/google/template/soy/jbcsrc/SoyNodeCompiler.java index 22ffe0ca1d..3ba0b9aa39 100644 --- a/java/src/com/google/template/soy/jbcsrc/SoyNodeCompiler.java +++ b/java/src/com/google/template/soy/jbcsrc/SoyNodeCompiler.java @@ -638,7 +638,7 @@ private Optional tryCompileSwitchToSwitchInstruction( asSwitchableInt(switchExpr, casesByKey.navigableKeySet()), casesByKey, defaultBlock)); } else { - // Otherwise we need more complex matching logic that we outsource to an invoke dyanmic + // Otherwise we need more complex matching logic that we outsource to an invoke dynamic // bootstrap. Create a fake key for each case and then rely on the bootstrap to figure it // out. // update the map with the pseudo keys, so that the loops below can find them @@ -843,7 +843,9 @@ protected Statement visitPrintNode(PrintNode node) { return visitLoggingFunction(node, fn, (LoggingFunction) fn.getSoyFunction()); } if (fn.getSoyFunction() == BuiltinFunction.FLUSH_PENDING_LOGGING_ATTRIBUTES) { - return appendableExpression.flushPendingLoggingAttributes().toStatement(); + return appendableExpression + .flushPendingLoggingAttributes(((BooleanNode) fn.getParams().get(0)).getValue()) + .toStatement(); } } // First check our special case where all print directives are streamable and an expression that @@ -1049,7 +1051,7 @@ protected Statement visitRawTextNode(RawTextNode node) { @Override protected Statement visitDebuggerNode(DebuggerNode node) { - // Call JbcSrcRuntime.debuggger. This logs a stack trace by default and is an obvious place to + // Call JbcSrcRuntime.debugger. This logs a stack trace by default and is an obvious place to // put a breakpoint. return MethodRefs.RUNTIME_DEBUGGER.invokeVoid( constant(node.getSourceLocation().getFilePath().path()), diff --git a/java/src/com/google/template/soy/jbcsrc/api/OutputAppendable.java b/java/src/com/google/template/soy/jbcsrc/api/OutputAppendable.java index 6226a85225..92e52624be 100644 --- a/java/src/com/google/template/soy/jbcsrc/api/OutputAppendable.java +++ b/java/src/com/google/template/soy/jbcsrc/api/OutputAppendable.java @@ -160,8 +160,10 @@ public LoggingAdvisingAppendable exitLoggableElement() { } @Override - public void flushBuffers(int depth) { - throw new AssertionError("shouldn't be called"); + public LoggingAdvisingAppendable flushPendingLoggingAttributes(boolean isAnchorTag) + throws IOException { + // TODO: b/383661457 - implement this. + return this; } private void appendDebugOutput(Optional veDebugOutput) { diff --git a/java/src/com/google/template/soy/jbcsrc/runtime/JbcSrcRuntime.java b/java/src/com/google/template/soy/jbcsrc/runtime/JbcSrcRuntime.java index ff97de441b..fa53278765 100644 --- a/java/src/com/google/template/soy/jbcsrc/runtime/JbcSrcRuntime.java +++ b/java/src/com/google/template/soy/jbcsrc/runtime/JbcSrcRuntime.java @@ -531,6 +531,11 @@ public LoggingAdvisingAppendable exitLoggableElement() { return this; } + @Override + public LoggingAdvisingAppendable flushPendingLoggingAttributes(boolean isAnchorTag) { + return this; + } + @Override public LoggingAdvisingAppendable appendLoggingFunctionInvocation( LoggingFunctionInvocation funCall, ImmutableList> escapers) { diff --git a/java/src/com/google/template/soy/passes/AddFlushPendingLoggingAttributesPass.java b/java/src/com/google/template/soy/passes/AddFlushPendingLoggingAttributesPass.java index e00d18c595..5173bf8466 100644 --- a/java/src/com/google/template/soy/passes/AddFlushPendingLoggingAttributesPass.java +++ b/java/src/com/google/template/soy/passes/AddFlushPendingLoggingAttributesPass.java @@ -20,6 +20,8 @@ import com.google.template.soy.base.internal.IdGenerator; import com.google.template.soy.base.internal.Identifier; import com.google.template.soy.error.ErrorReporter; +import com.google.template.soy.exprtree.BooleanNode; +import com.google.template.soy.exprtree.ExprNode; import com.google.template.soy.exprtree.FunctionNode; import com.google.template.soy.shared.internal.BuiltinFunction; import com.google.template.soy.soytree.HtmlAttributeNode; @@ -92,6 +94,7 @@ private void instrumentNode(IdGenerator nodeIdGen, HtmlOpenTagNode openTag) { BuiltinFunction.FLUSH_PENDING_LOGGING_ATTRIBUTES.name(), SourceLocation.UNKNOWN), BuiltinFunction.FLUSH_PENDING_LOGGING_ATTRIBUTES, SourceLocation.UNKNOWN); + functionCall.addChild(getTagIsAnchorNode(openTag)); functionCall.setType(AttributesType.getInstance()); var printNode = new PrintNode( @@ -111,4 +114,12 @@ private void instrumentNode(IdGenerator nodeIdGen, HtmlOpenTagNode openTag) { attributeNode.addChild(printNode); openTag.addChild(attributeNode); } + + private static ExprNode getTagIsAnchorNode(HtmlOpenTagNode openTag) { + var tagName = openTag.getTagName(); + return new BooleanNode( + // If the tag is dynamic we assume it isn't an anchor + tagName.isStatic() && tagName.getStaticTagNameAsLowerCase().equals("a"), + tagName.getTagLocation()); + } } diff --git a/java/src/com/google/template/soy/shared/internal/BuiltinFunction.java b/java/src/com/google/template/soy/shared/internal/BuiltinFunction.java index 11bb23a9b5..ac704313e4 100644 --- a/java/src/com/google/template/soy/shared/internal/BuiltinFunction.java +++ b/java/src/com/google/template/soy/shared/internal/BuiltinFunction.java @@ -20,6 +20,7 @@ import com.google.common.collect.ImmutableSet; import com.google.template.soy.shared.restricted.SoyFunction; import com.google.template.soy.types.AnyType; +import com.google.template.soy.types.BoolType; import com.google.template.soy.types.IterableType; import com.google.template.soy.types.NullType; import com.google.template.soy.types.SanitizedType; @@ -93,7 +94,6 @@ public Set getValidArgsSizes() { case IS_PRIMARY_MSG_IN_USE: return ImmutableSet.of(3); case DEBUG_SOY_TEMPLATE_INFO: - case FLUSH_PENDING_LOGGING_ATTRIBUTES: return ImmutableSet.of(0); case SOY_SERVER_KEY: case CHECK_NOT_NULL: @@ -110,6 +110,7 @@ public Set getValidArgsSizes() { case IS_TRUTHY_NON_EMPTY: case HAS_CONTENT: case NEW_SET: + case FLUSH_PENDING_LOGGING_ATTRIBUTES: return ImmutableSet.of(1); case PROTO_INIT: throw new UnsupportedOperationException(); @@ -135,6 +136,8 @@ public Optional> getValidArgTypes() { StringType.getInstance(), NullType.getInstance(), UndefinedType.getInstance()))); + case FLUSH_PENDING_LOGGING_ATTRIBUTES: + return Optional.of(ImmutableList.of(BoolType.getInstance())); case NEW_SET: // This is further constrained in ResolveExpressionTypesPass. return Optional.of(ImmutableList.of(IterableType.of(AnyType.getInstance()))); diff --git a/java/src/com/google/template/soy/shared/internal/Sanitizers.java b/java/src/com/google/template/soy/shared/internal/Sanitizers.java index 75b7576df7..e95fe69aef 100644 --- a/java/src/com/google/template/soy/shared/internal/Sanitizers.java +++ b/java/src/com/google/template/soy/shared/internal/Sanitizers.java @@ -239,6 +239,16 @@ public LoggingAdvisingAppendable exitLoggableElement() { return this; } + @CanIgnoreReturnValue + @Override + public LoggingAdvisingAppendable flushPendingLoggingAttributes(boolean isAnchorTag) + throws IOException { + if (isInHtml()) { + delegate.flushPendingLoggingAttributes(isAnchorTag); + } + return this; + } + @CanIgnoreReturnValue @Override public LoggingAdvisingAppendable appendLoggingFunctionInvocation( @@ -329,6 +339,11 @@ public LoggingAdvisingAppendable enterLoggableElement(LogStatement statement) { public LoggingAdvisingAppendable exitLoggableElement() { return this; } + + @Override + public LoggingAdvisingAppendable flushPendingLoggingAttributes(boolean isAnchorTag) { + return this; + } } /** Normalizes HTML to HTML making sure quotes and other specials are entity encoded. */ @@ -962,6 +977,11 @@ public LoggingAdvisingAppendable exitLoggableElement() { return this; } + @Override + public LoggingAdvisingAppendable flushPendingLoggingAttributes(boolean isAnchorTag) { + return this; + } + @CanIgnoreReturnValue @Override public LoggingAdvisingAppendable appendLoggingFunctionInvocation( @@ -1149,6 +1169,16 @@ public LoggingAdvisingAppendable appendLoggingFunctionInvocation( return this; } + @CanIgnoreReturnValue + @Override + public LoggingAdvisingAppendable flushPendingLoggingAttributes(boolean isAnchorTag) + throws IOException { + if (getSanitizedContentKind() == ContentKind.ATTRIBUTES) { + delegate.flushPendingLoggingAttributes(isAnchorTag); + } + return this; + } + @Override public boolean softLimitReached() { return delegate.softLimitReached(); diff --git a/java/src/com/google/template/soy/shared/internal/StreamingAttributeEscaper.java b/java/src/com/google/template/soy/shared/internal/StreamingAttributeEscaper.java index cdcf604155..8a3aed30d6 100644 --- a/java/src/com/google/template/soy/shared/internal/StreamingAttributeEscaper.java +++ b/java/src/com/google/template/soy/shared/internal/StreamingAttributeEscaper.java @@ -127,6 +127,17 @@ public LoggingAdvisingAppendable appendLoggingFunctionInvocation( return this; } + @CanIgnoreReturnValue + @Override + public LoggingAdvisingAppendable flushPendingLoggingAttributes(boolean isAnchorTag) + throws IOException { + var buffer = this.buffer; + if (buffer == null) { + delegate.flushPendingLoggingAttributes(isAnchorTag); + } + return this; + } + @Override public boolean softLimitReached() { return delegate.softLimitReached(); diff --git a/java/src/com/google/template/soy/shared/internal/StreamingEscaper.java b/java/src/com/google/template/soy/shared/internal/StreamingEscaper.java index 657a2d128a..5e494690ab 100644 --- a/java/src/com/google/template/soy/shared/internal/StreamingEscaper.java +++ b/java/src/com/google/template/soy/shared/internal/StreamingEscaper.java @@ -87,6 +87,11 @@ public LoggingAdvisingAppendable appendLoggingFunctionInvocation( return append(escapePlaceholder(funCall.placeholderValue(), escapers)); } + @Override + public LoggingAdvisingAppendable flushPendingLoggingAttributes(boolean isAnchorTag) { + return this; + } + @Override public boolean softLimitReached() { return delegate.softLimitReached(); diff --git a/java/tests/com/google/template/soy/jbcsrc/StreamingPrintDirectivesTest.java b/java/tests/com/google/template/soy/jbcsrc/StreamingPrintDirectivesTest.java index ce25c3e202..bee159d153 100644 --- a/java/tests/com/google/template/soy/jbcsrc/StreamingPrintDirectivesTest.java +++ b/java/tests/com/google/template/soy/jbcsrc/StreamingPrintDirectivesTest.java @@ -37,7 +37,6 @@ import com.google.template.soy.data.SoyRecord; import com.google.template.soy.data.SoyValueConverter; import com.google.template.soy.data.SoyValueConverterUtility; -import com.google.template.soy.data.SoyValueProvider; import com.google.template.soy.data.internal.ParamStore; import com.google.template.soy.error.ErrorReporter; import com.google.template.soy.jbcsrc.restricted.BytecodeUtils; @@ -458,6 +457,13 @@ public LoggingAdvisingAppendable exitLoggableElement() { return this; } + @Override + public LoggingAdvisingAppendable flushPendingLoggingAttributes(boolean isAnchorTag) + throws IOException { + delegate.flushPendingLoggingAttributes(isAnchorTag); + return this; + } + @CanIgnoreReturnValue @Override public LoggingAdvisingAppendable appendLoggingFunctionInvocation(