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

Escaping correctly carriage returns #41

Open
mebibou opened this issue Apr 3, 2013 · 12 comments
Open

Escaping correctly carriage returns #41

mebibou opened this issue Apr 3, 2013 · 12 comments

Comments

@mebibou
Copy link

mebibou commented Apr 3, 2013

I'm using a modified version of PlayLessPlugin for my needs on a project that is also using Press. Since I needed to be able to get the compiled output of a file as a String, I create a method in PlayLessEngine to do it, and name my class MyPlayLessEngine.

The problem that I'm having now is that after using MyPlayLessEngine on an HTML page, and going on a different Html page that is using the press plugin, then '\n' are inserted inside the pressed Less files, therefore causing the css output to be wrong and not taken into my page.

So I added a package named press in my code, with a PlayLessEngine within to override the default one, and instead of doing return css.replace("\\n", "\n"); on line 129 I do return css.replace("\\n", "").replace("\n", ""); and then I don't have the problem anymore. Has it been happening to anyone? would there be another way to correct this? thanks

@dirkmc
Copy link
Owner

dirkmc commented Apr 3, 2013

The less lib was just updated recently, maybe that's causing problems?

@mebibou
Copy link
Author

mebibou commented Apr 4, 2013

I'm not sure, what version should I try then?

@dirkmc
Copy link
Owner

dirkmc commented Apr 4, 2013

I think we should stick with the latest, I'm just saying that maybe something changed in that lib that is causing the problem... so maybe that's a place to start investigating. This \n issue is tricky, I remember struggling with it before eventually adding this hack to the code.

@mebibou
Copy link
Author

mebibou commented Apr 4, 2013

Ok so in this case maybe the replacement for "\n" that you added in your code could be replaced by a configurable key instead of "\n", so that people that have the same problem as I do could correct it by just replacing "\n" by nothing, don't you think?

@dirkmc
Copy link
Owner

dirkmc commented Apr 4, 2013

I'm not sure if I understand the problem you're having. Are you saying that in the resulting less code you see a backslash character and an n character, or are you saying you see newlines?

@mebibou
Copy link
Author

mebibou commented Apr 4, 2013

Yes, in the compress files we see the backslash character as it is, they do not appear as new lines, so for exemple I have body {\n} in the output when I shouldn't see it.

The thing is that we are using another modified version of the PlayLessEngine somewhere else because we need to be able to access the protected function protected String compile(File lessFile, boolean compress) {...}, but I renamed the file as MyPlayLessEngine so that it wouldn't interfere with yours. So I'm not sure if it's linked, but after a few calls to the plugin on different html pages the outputs contain backslash characters and the css files cannot be correctly parsed anymore

@dirkmc
Copy link
Owner

dirkmc commented Apr 4, 2013

Ahhh, ok yeah I think that's most likely your problem. One of the libs included by play-less is probably overwriting the one included by play-press. Try removing the dependency on play-less and also remove play-less from the modules directory in your play installation.

@mebibou
Copy link
Author

mebibou commented Apr 5, 2013

Thanks for the replies. Unfortunately after removing the the other module that was using also the play-less plugin, I just had the error again (which I did not have for some time but appeared again). Here's what the bootstrap.less file looks like :
`/*!

and my dependency file looking like
require: - play - play -> logisimayml 1.8 - play -> secure - play -> cobertura 2.4 - org.codehaus.jackson -> jackson-mapper-asl 1.9.10 - org.concordion -> concordion 1.4.2 - com.microsoft -> jdbc4 1.0 - org.hibernate -> hibernate-entitymanager 4.1.6.Final - nl.siegmann.epublib -> epublib-core 3.1 - eu.medsea.mimeutil -> mime-util 2.1.3 - org.apache.commons -> commons-lang3 3.1 - org.json -> json 20090211 - play -> press 1.0.36 - play -> messages 1.3

@dirkmc
Copy link
Owner

dirkmc commented Apr 5, 2013

hmmm..
Are you sure you removed the less module from play's cache in play/modules?

@mebibou
Copy link
Author

mebibou commented Apr 15, 2013

Yes i removed it, nowhere to be found, and it is still happening... so now when it happens I just modified one line in LessCompiler.java and iI reload and it works, but not very good for production environnement!

@dirkmc
Copy link
Owner

dirkmc commented Apr 15, 2013

You said "when it happens". Does it not happen every time?
Could you tell me exactly what version of play you are running, and what versions of each module you have loaded. I'll try to reproduce it and see if I can work out what's causing it.

@mebibou
Copy link
Author

mebibou commented Apr 22, 2013

No it does not happen all the time, and I couldn't find a way to reproduce it, it just does sometimes, and then either I restart my play server or I change the the file that makes it buggy.
I'm running play-1.2.5, and the modules are listed as above in my dependency.yml.

Other than that, I'm using this version of the PlayLessEngine to access the public String compile(...) function:

package utils;

import java.io.BufferedReader;
import java.io.File;
import java.io.FileNotFoundException;
import java.io.FileReader;
import java.io.IOException;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import org.apache.commons.io.IOUtils;
import org.mozilla.javascript.WrappedException;

import play.Logger;
import play.cache.Cache;

import com.asual.lesscss.LessEngine;
import com.asual.lesscss.LessException;

public class MyPlayLessEngine {

  LessEngine lessEngine;
  Boolean devMode;
  Pattern importPattern = Pattern.compile(".*@import\\s*\"(.*?)\".*");

  MyPlayLessEngine(Boolean devMode) {
    lessEngine = new LessEngine();
    this.devMode = devMode;
  }

  /**
   * Get the CSS for this less file either from the cache, or compile it.
   */
  public String get(File lessFile) {
    String cacheKey = "less_" + lessFile.getPath() + lastModifiedRecursive(lessFile);
    String css = cacheGet(cacheKey, String.class);
    if (css == null) {
      css = compile(lessFile);
      cacheSet(cacheKey, css);
    }
    return css;
  }

  // TODO: Maybe prevent infinite looping here, in case of an import loop?
  public long lastModifiedRecursive(File lessFile) {
    long lastModified = lessFile.lastModified();
    for (File imported : getImportsFromCacheOrFile(lessFile)) {
      lastModified = Math.max(lastModified, imported.lastModified());
    }
    return lastModified;
  }

  protected Set<File> getImportsFromCacheOrFile(File lessFile) {
    String cacheKey = "less_imports_" + lessFile.getPath() + lessFile.lastModified();

    Set<File> files = null;
    cacheGet(cacheKey, Set.class);

    if (files == null) {
      try {
        files = getImportsFromFile(lessFile);
        cacheSet(cacheKey, files);
      } catch (IOException e) {
        Logger.error(e, "IOException trying to determine imports in LESS file");
        files = new HashSet<File>();
      }
    }
    return files;
  }

  protected Set<File> getImportsFromFile(File lessFile) throws IOException {
    if (!lessFile.exists()) {
      return Collections.emptySet();
    }

    BufferedReader r = new BufferedReader(new FileReader(lessFile));
    try {
      Set<File> files = new HashSet<File>();
      String line;
      while ((line = r.readLine()) != null) {
        Matcher m = importPattern.matcher(line);
        while (m.find()) {
          File file = new File(lessFile.getParentFile(), m.group(1));
          if (!file.exists())
            file = new File(lessFile.getParentFile(), m.group(1) + ".less");
          files.add(file);
          files.addAll(getImportsFromCacheOrFile(file));
        }
      }
      return files;
    } finally {
      IOUtils.closeQuietly(r);
    }
  }

  public String compile(String input) throws LessException {
    return lessEngine.compile(input);
  }

  protected String compile(File lessFile) {
    try {
      return lessEngine.compile(lessFile);
    } catch (LessException e) {
      return handleException(lessFile, e);
    }
  }

  public String handleException(File lessFile, LessException e) {
    Logger.warn(e, "Less exception");

    String filename = e.getFilename();
    List<String> extractList = e.getExtract();
    String extract = null;
    if (extractList != null) {
      extract = extractList.toString();
    }

    // LessEngine reports the file as null when it's not an @imported file
    if (filename == null) {
      filename = lessFile.getName();
    }

    // Try to detect missing imports (flaky)
    if (extract == null && e.getCause() instanceof WrappedException) {
      WrappedException we = (WrappedException) e.getCause();
      if (we.getCause() instanceof FileNotFoundException) {
        FileNotFoundException fnfe = (FileNotFoundException) we.getCause();
        extract = fnfe.getMessage();
      }
    }

    return formatMessage(filename, e.getLine(), e.getColumn(), extract, e.getType());
  }

  public String formatMessage(String filename, int line, int column, String extract, String errorType) {
    return "body:before {display: block; color: #c00; white-space: pre; font-family: monospace; background: #FDD9E1; border-top: 1px solid pink; border-bottom: 1px solid pink; padding: 10px; content: \"[LESS ERROR] " + String.format("%s:%s: %s (%s)", filename, line, extract, errorType) + "\"; }";
  }

  private static <T> T cacheGet(String key, Class<T> clazz) {
    try {
      return Cache.get(key, clazz);
    } catch (NullPointerException e) {
      Logger.info("LESS module: Cache not initialized yet. Request to regular action required to initialize cache in DEV mode.");
      return null;
    }
  }

  private static void cacheSet(String key, Object value) {
    try {
      Cache.set(key, value);
    } catch (NullPointerException e) {
      Logger.info("LESS module: Cache not initialized yet. Request to regular action required to initialize cache in DEV mode.");
    }
  }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants