Then that is my fault and I apologize.

Then again, that is really the only reason why anybody would recommend 
refactoring code so it wasn't an out of nowhere assumption.

This is starting to sound like the suhosin way of "security".  Which 
is: "somebody can potentially write code that *might* be possibly 
dangerous in some circumstances, so the solution is to prevent that 
code from ever being written."  (Suhosin for C would allow you to do 
nothing more than output static strings, since memory allocation can 
possibly be dangerous when done by people who don't know what they are 
doing, so we can't allow that.)
Disagree from a security perspective (for our project).  Agree from a 
performance/cleaner design perspective (for our project).

This requires another connection to the server.  Can't assume this 
code is cached.  Can't assume you have a fast network connection.  So 
this is not  equivalent replacement code.

Going to close.  eval() with autocomplete is going away (or has 
already been removed - can't remember), since server interaction 
paradigm has changed in git master.  Other existing eval() code is 
fine for reasons I've stated previously.

I certainly did not claim that. I don't know where you see such a 
claim in my bugreport.
Uhm, I am a bit baffled, as I did not expect this kind of argument.

Of course the use of eval is not necessarily dangerous, in the same 
way it is perfectly safe for skilled people to swallow a sword. It is 
however pretty easy to make a mistake and face severe consequences.

That is why i would consider passing a variable x to eval more 
dangerous than passing it to lets say split(). Since if through some 
other problems one is able to control x, the consequences are much 
more severe if x is evald.

Of course not using eval is not some magic potion, but it is certainly 
a step in the right direction in my opinion.
That was exactly my proposal.
Uhm how about:
document.createElement('script').src = '/myShinyNewScript.js';
I see your argument and certainly it looks very similar if you receive 
code from the server via a script tag, or via ajax+eval.

But as I said it is a preventive measure. It makes it less likely that 
simple bugs can be escalated to security bugs if you separate data and 
code. If your php api is returning a json object, it might not 
immediately be clear to the backend coder, that parts of that object 
will be evald for example.

Well it certainly is your call on how to design your software, this 
was meant as a friendly hint...

Use of eval() is perfectly acceptable.  I see way too many people who 
say things like "eval() should NEVER be used".  Which is flat-out 
wrong.  eval() is no more dangerous than anything else - meaning it 
can be abused if used incorrectly.

Nobody has shown that our code is subject to any security issue.
Sure.  And what's wrong with this?

We shouldn't be loading this code at page load time, since the user 
may never use this code.  We should only run it on demand.  (This is 
the tradeoff for trying to separate the code layer from the display 
layer.)
Easier to read.  Brackets are difficult when viewing embedded in PHP code.
Realize that there is a bunch of javascript library APIs that we 
currently use that require this kind of dynamic code.  I believe 
autocomplete is an example of this.  So this can't be removed (at 
least for H5).

I'm not saying that removing eval() is not something we should strive 
for from a *design* perspective.  But I'm not sure what your 
alternative is.  There is no difference, security wise, between 
separate script files and eval'd code, as long as the eval'd code is 
properly escaped.  And I really don't want to have to lump all 
javascript code into a single bundle when it is likely that the code 
is never needed on the page.

We are in the process of deploying csp headers for horde. Through that 
we discovered usage of js eval in horde. Especially for a webmail and 
the associated danger of injections it would be nice if horde could 
discontinue the use of eval (and maybe even inline js/css).

One particular case we see is in DimpBase>>loadPreview. It seems it is 
only used atm to display modal dialogs from 
horde/imp/lib/Ajax/Imple/PassphraseDialog.php.

Then in the dynamic composer:
new Function("t", "return t.sub(/<[^>]*>$/, 
\"\").strip().escapeHTML()") afaik this is a static string, so why 
even new Function?

Then there are some reports from 
/imp/dynamic.php?page=message&mailbox=... that I was not able to 
tackle yet.

There might be more occurrences.

If you are interested in fixing those, we can provide more data as 
soon as we have better processing for the logs.